fix(admin): Rename secureModeEnabled to strictModeDisabled (#17789)

This commit is contained in:
Shreya Keshive
2026-01-29 10:31:00 -05:00
committed by GitHub
parent 42eedc9184
commit f619cd8750
4 changed files with 70 additions and 33 deletions

View File

@@ -2156,7 +2156,7 @@ describe('Settings Loading and Merging', () => {
// 2. Now, set remote admin settings.
loadedSettings.setRemoteAdminSettings({
secureModeEnabled: true,
strictModeDisabled: false,
mcpSetting: { mcpEnabled: false },
cliFeatureSetting: { extensionsSetting: { extensionsEnabled: false } },
});
@@ -2197,7 +2197,7 @@ describe('Settings Loading and Merging', () => {
expect(loadedSettings.merged.ui?.theme).toBe('initial-theme');
const newRemoteSettings = {
secureModeEnabled: true,
strictModeDisabled: false,
mcpSetting: { mcpEnabled: false },
cliFeatureSetting: { extensionsSetting: { extensionsEnabled: false } },
};
@@ -2213,7 +2213,7 @@ describe('Settings Loading and Merging', () => {
// Verify that calling setRemoteAdminSettings with partial data overwrites previous remote settings
// and missing properties revert to schema defaults.
loadedSettings.setRemoteAdminSettings({ secureModeEnabled: false });
loadedSettings.setRemoteAdminSettings({ strictModeDisabled: true });
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false);
expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(false); // Defaulting to false if missing
expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false); // Defaulting to false if missing
@@ -2271,9 +2271,9 @@ describe('Settings Loading and Merging', () => {
expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(true);
expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(true);
// Set remote settings with only secureModeEnabled
// Set remote settings with only strictModeDisabled (false -> secureModeEnabled: true)
loadedSettings.setRemoteAdminSettings({
secureModeEnabled: true,
strictModeDisabled: false,
});
// Verify secureModeEnabled is updated, others default to false
@@ -2286,8 +2286,8 @@ describe('Settings Loading and Merging', () => {
mcpSetting: { mcpEnabled: false },
});
// Verify mcpEnabled is updated, others remain defaults (secureModeEnabled reverts to default:false)
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false);
// Verify mcpEnabled is updated, others remain defaults (secureModeEnabled defaults to true if strictModeDisabled is missing)
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true);
expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(false);
expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false);
@@ -2297,9 +2297,33 @@ describe('Settings Loading and Merging', () => {
});
// Verify extensionsEnabled is updated, others remain defaults
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false);
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true);
expect(loadedSettings.merged.admin?.mcp?.enabled).toBe(false);
expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false);
// Verify that missing strictModeDisabled falls back to secureModeEnabled
loadedSettings.setRemoteAdminSettings({
secureModeEnabled: false,
});
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false);
loadedSettings.setRemoteAdminSettings({
secureModeEnabled: true,
});
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true);
// Verify strictModeDisabled takes precedence over secureModeEnabled
loadedSettings.setRemoteAdminSettings({
strictModeDisabled: false,
secureModeEnabled: false,
});
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true);
loadedSettings.setRemoteAdminSettings({
strictModeDisabled: true,
secureModeEnabled: true,
});
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false);
});
it('should set skills based on unmanagedCapabilitiesEnabled', () => {
@@ -2337,12 +2361,12 @@ describe('Settings Loading and Merging', () => {
expect(loadedSettings.merged.admin?.extensions?.enabled).toBe(false);
});
it('should force secureModeEnabled to false if undefined, overriding schema defaults', () => {
// Mock schema to have secureModeEnabled default to true to verify the override
it('should force secureModeEnabled to true if undefined, overriding schema defaults', () => {
// Mock schema to have secureModeEnabled default to false to verify the override
const originalSchema = getSettingsSchema();
const modifiedSchema = JSON.parse(JSON.stringify(originalSchema));
if (modifiedSchema.admin?.properties?.secureModeEnabled) {
modifiedSchema.admin.properties.secureModeEnabled.default = true;
modifiedSchema.admin.properties.secureModeEnabled.default = false;
}
vi.mocked(getSettingsSchema).mockReturnValue(modifiedSchema);
@@ -2352,13 +2376,13 @@ describe('Settings Loading and Merging', () => {
const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR);
// Pass a non-empty object that doesn't have secureModeEnabled
// Pass a non-empty object that doesn't have strictModeDisabled
loadedSettings.setRemoteAdminSettings({
mcpSetting: {},
});
// It should be forced to false by the logic, overriding the mock default of true
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(false);
// It should be forced to true by the logic (default secure), overriding the mock default of false
expect(loadedSettings.merged.admin?.secureModeEnabled).toBe(true);
} finally {
vi.mocked(getSettingsSchema).mockReturnValue(originalSchema);
}

View File

@@ -348,7 +348,12 @@ export class LoadedSettings {
setRemoteAdminSettings(remoteSettings: FetchAdminControlsResponse): void {
const admin: Settings['admin'] = {};
const { secureModeEnabled, mcpSetting, cliFeatureSetting } = remoteSettings;
const {
secureModeEnabled,
strictModeDisabled,
mcpSetting,
cliFeatureSetting,
} = remoteSettings;
if (Object.keys(remoteSettings).length === 0) {
this._remoteAdminSettings = { admin };
@@ -356,7 +361,13 @@ export class LoadedSettings {
return;
}
admin.secureModeEnabled = secureModeEnabled ?? false;
if (strictModeDisabled !== undefined) {
admin.secureModeEnabled = !strictModeDisabled;
} else if (secureModeEnabled !== undefined) {
admin.secureModeEnabled = secureModeEnabled;
} else {
admin.secureModeEnabled = true;
}
admin.mcp = { enabled: mcpSetting?.mcpEnabled ?? false };
admin.extensions = {
enabled: cliFeatureSetting?.extensionsSetting?.extensionsEnabled ?? false,

View File

@@ -44,7 +44,7 @@ describe('Admin Controls', () => {
describe('sanitizeAdminSettings', () => {
it('should strip unknown fields', () => {
const input = {
secureModeEnabled: true,
strictModeDisabled: false,
extraField: 'should be removed',
mcpSetting: {
mcpEnabled: false,
@@ -55,7 +55,7 @@ describe('Admin Controls', () => {
const result = sanitizeAdminSettings(input);
expect(result).toEqual({
secureModeEnabled: true,
strictModeDisabled: false,
mcpSetting: {
mcpEnabled: false,
},
@@ -104,7 +104,7 @@ describe('Admin Controls', () => {
});
it('should use cachedSettings and start polling if provided', async () => {
const cachedSettings = { secureModeEnabled: true };
const cachedSettings = { strictModeDisabled: false };
const result = await fetchAdminControls(
mockServer,
cachedSettings,
@@ -117,7 +117,7 @@ describe('Admin Controls', () => {
// Should still start polling
(mockServer.fetchAdminControls as Mock).mockResolvedValue({
secureModeEnabled: false,
strictModeDisabled: true,
});
await vi.advanceTimersByTimeAsync(5 * 60 * 1000);
@@ -136,7 +136,7 @@ describe('Admin Controls', () => {
});
it('should fetch from server if no cachedSettings provided', async () => {
const serverResponse = { secureModeEnabled: true };
const serverResponse = { strictModeDisabled: false };
(mockServer.fetchAdminControls as Mock).mockResolvedValue(serverResponse);
const result = await fetchAdminControls(
@@ -164,7 +164,7 @@ describe('Admin Controls', () => {
// Polling should have been started and should retry
(mockServer.fetchAdminControls as Mock).mockResolvedValue({
secureModeEnabled: true,
strictModeDisabled: false,
});
await vi.advanceTimersByTimeAsync(5 * 60 * 1000);
expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(2); // Initial + poll
@@ -191,7 +191,7 @@ describe('Admin Controls', () => {
it('should sanitize server response', async () => {
(mockServer.fetchAdminControls as Mock).mockResolvedValue({
secureModeEnabled: true,
strictModeDisabled: false,
unknownField: 'bad',
});
@@ -201,7 +201,7 @@ describe('Admin Controls', () => {
true,
mockOnSettingsChanged,
);
expect(result).toEqual({ secureModeEnabled: true });
expect(result).toEqual({ strictModeDisabled: false });
expect(
(result as Record<string, unknown>)['unknownField'],
).toBeUndefined();
@@ -245,7 +245,7 @@ describe('Admin Controls', () => {
it('should poll and emit changes', async () => {
// Initial fetch
(mockServer.fetchAdminControls as Mock).mockResolvedValue({
secureModeEnabled: false,
strictModeDisabled: true,
});
await fetchAdminControls(
mockServer,
@@ -256,19 +256,19 @@ describe('Admin Controls', () => {
// Update for next poll
(mockServer.fetchAdminControls as Mock).mockResolvedValue({
secureModeEnabled: true,
strictModeDisabled: false,
});
// Fast forward
await vi.advanceTimersByTimeAsync(5 * 60 * 1000);
expect(mockOnSettingsChanged).toHaveBeenCalledWith({
secureModeEnabled: true,
strictModeDisabled: false,
});
});
it('should NOT emit if settings are deeply equal but not the same instance', async () => {
const settings = { secureModeEnabled: true };
const settings = { strictModeDisabled: false };
(mockServer.fetchAdminControls as Mock).mockResolvedValue(settings);
await fetchAdminControls(
@@ -282,7 +282,7 @@ describe('Admin Controls', () => {
// Next poll returns a different object with the same values
(mockServer.fetchAdminControls as Mock).mockResolvedValue({
secureModeEnabled: true,
strictModeDisabled: false,
});
await vi.advanceTimersByTimeAsync(5 * 60 * 1000);
@@ -293,7 +293,7 @@ describe('Admin Controls', () => {
it('should continue polling after a fetch error', async () => {
// Initial fetch is successful
(mockServer.fetchAdminControls as Mock).mockResolvedValue({
secureModeEnabled: false,
strictModeDisabled: true,
});
await fetchAdminControls(
mockServer,
@@ -313,19 +313,19 @@ describe('Admin Controls', () => {
// Subsequent poll succeeds with new data
(mockServer.fetchAdminControls as Mock).mockResolvedValue({
secureModeEnabled: true,
strictModeDisabled: false,
});
await vi.advanceTimersByTimeAsync(5 * 60 * 1000);
expect(mockServer.fetchAdminControls).toHaveBeenCalledTimes(3);
expect(mockOnSettingsChanged).toHaveBeenCalledWith({
secureModeEnabled: true,
strictModeDisabled: false,
});
});
it('should STOP polling if server returns 403', async () => {
// Initial fetch is successful
(mockServer.fetchAdminControls as Mock).mockResolvedValue({
secureModeEnabled: false,
strictModeDisabled: true,
});
await fetchAdminControls(
mockServer,

View File

@@ -317,7 +317,9 @@ const McpSettingSchema = z.object({
});
export const FetchAdminControlsResponseSchema = z.object({
// TODO: deprecate once backend stops sending this field
secureModeEnabled: z.boolean().optional(),
strictModeDisabled: z.boolean().optional(),
mcpSetting: McpSettingSchema.optional(),
cliFeatureSetting: CliFeatureSettingSchema.optional(),
});