mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-02 13:40:46 +00:00
feat(cli): add slashCommands.disabled setting to gate slash commands (#3445)
* feat(cli): add slashCommands.disabled setting to gate slash commands
Introduces a first-class way for operators to hide and refuse to execute
specific slash commands. Useful for multi-tenant / enterprise / sandboxed
deployments where different users should see different command subsets.
The denylist is sourced from three unioned inputs:
* `slashCommands.disabled` settings key (string[], UNION merge), so
workspace scopes can only add to a denylist set at user or system
scope, never shrink it — matching the shape already used by
`permissions.deny`.
* `--disabled-slash-commands` CLI flag (comma-separated or repeated).
* `QWEN_DISABLED_SLASH_COMMANDS` environment variable.
Matching is case-insensitive against the final (post-rename) command
name, so extension commands are addressable by their disambiguated
form (e.g. `firebase.deploy`). Disabled commands are removed from
`CommandService`'s output, so they disappear from autocomplete and
produce the standard unknown-command path in both interactive TUI and
non-interactive (`--prompt`) modes.
The scope of this change is slash commands only: it does not affect
tool permissions (still `permissions.deny`) or keyboard shortcuts.
* chore(cli): regenerate settings.schema.json for slashCommands.disabled
Regenerates the companion JSON schema consumed by the VS Code extension
after adding the `slashCommands.disabled` entry to the TS schema in the
previous commit. Required by the "Check settings schema is up-to-date"
CI lint step.
* fix(cli): route disabled slash commands to unsupported, not no_command
handleSlashCommand was passing the disabled denylist straight into
CommandService.create, so disabled commands disappeared from
`allCommands` too. The fallback existence check that distinguishes
"known but not allowed in non-interactive mode" from "truly unknown"
then failed, and disabled commands like `/help` fell through to
`no_command` — causing the caller to forward them to the model as
plain prompt text.
Keep `allCommands` unfiltered and apply the denylist only when
constructing the executable set and when producing the unsupported
response. A disabled command now returns `unsupported` with a
"disabled by the current configuration" reason and never reaches the
model. Added three regression tests covering the primary case,
case-insensitive match, and the preserved no_command path for
genuinely unknown input.
This commit is contained in:
parent
7cded6e0df
commit
0b8b3da836
14 changed files with 398 additions and 41 deletions
|
|
@ -106,6 +106,7 @@ export async function handleQwenAuth(
|
|||
maxSessionTurns: undefined,
|
||||
coreTools: undefined,
|
||||
excludeTools: undefined,
|
||||
disabledSlashCommands: undefined,
|
||||
authType: undefined,
|
||||
channel: undefined,
|
||||
systemPrompt: undefined,
|
||||
|
|
|
|||
|
|
@ -160,6 +160,7 @@ export interface CliArgs {
|
|||
maxSessionTurns: number | undefined;
|
||||
coreTools: string[] | undefined;
|
||||
excludeTools: string[] | undefined;
|
||||
disabledSlashCommands: string[] | undefined;
|
||||
authType: string | undefined;
|
||||
channel: string | undefined;
|
||||
jsonFd?: number | undefined;
|
||||
|
|
@ -530,6 +531,17 @@ export async function parseArguments(): Promise<CliArgs> {
|
|||
coerce: (tools: string[]) =>
|
||||
tools.flatMap((tool) => tool.split(',').map((t) => t.trim())),
|
||||
})
|
||||
.option('disabled-slash-commands', {
|
||||
type: 'array',
|
||||
string: true,
|
||||
description:
|
||||
'Slash command names to hide/disable (comma-separated or ' +
|
||||
'repeated). Merged with the `slashCommands.disabled` setting ' +
|
||||
'and QWEN_DISABLED_SLASH_COMMANDS. Matched case-insensitively ' +
|
||||
'against the final command name.',
|
||||
coerce: (names: string[]) =>
|
||||
names.flatMap((n) => n.split(',').map((t) => t.trim())),
|
||||
})
|
||||
.option('auth-type', {
|
||||
type: 'string',
|
||||
choices: [
|
||||
|
|
@ -926,6 +938,29 @@ export async function loadCliConfig(
|
|||
if (t && !mergedDeny.includes(t)) mergedDeny.push(t);
|
||||
}
|
||||
|
||||
// Merge the slash-command denylist from settings + CLI flag + env var.
|
||||
// Settings merge (UNION across scopes) is already handled upstream; we
|
||||
// only de-duplicate while preserving case for diagnostic purposes.
|
||||
const disabledSlashCommands: string[] = [];
|
||||
const seenDisabled = new Set<string>();
|
||||
const addDisabled = (value: string | undefined) => {
|
||||
if (!value) return;
|
||||
const trimmed = value.trim();
|
||||
if (!trimmed) return;
|
||||
const key = trimmed.toLowerCase();
|
||||
if (!seenDisabled.has(key)) {
|
||||
seenDisabled.add(key);
|
||||
disabledSlashCommands.push(trimmed);
|
||||
}
|
||||
};
|
||||
for (const name of settings.slashCommands?.disabled ?? []) addDisabled(name);
|
||||
for (const name of argv.disabledSlashCommands ?? []) addDisabled(name);
|
||||
for (const name of (process.env['QWEN_DISABLED_SLASH_COMMANDS'] ?? '').split(
|
||||
',',
|
||||
)) {
|
||||
addDisabled(name);
|
||||
}
|
||||
|
||||
// Helper: check if a tool is explicitly covered by an allow rule OR by the
|
||||
// coreTools whitelist. Uses alias matching for coreTools (via isToolEnabled)
|
||||
// to preserve the original behaviour where "ShellTool", "Shell", and
|
||||
|
|
@ -1093,6 +1128,8 @@ export async function loadCliConfig(
|
|||
? argv.allowedTools || undefined
|
||||
: argv.allowedTools || settings.tools?.allowed || undefined,
|
||||
excludeTools: mergedDeny,
|
||||
disabledSlashCommands:
|
||||
disabledSlashCommands.length > 0 ? disabledSlashCommands : undefined,
|
||||
// New unified permissions (PermissionManager source of truth).
|
||||
permissions: {
|
||||
allow: mergedAllow.length > 0 ? mergedAllow : undefined,
|
||||
|
|
|
|||
|
|
@ -954,6 +954,34 @@ describe('Settings Loading and Merging', () => {
|
|||
expect(settings.merged.advanced?.excludedEnvVars).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('should UNION-merge slashCommands.disabled across user and workspace scopes', () => {
|
||||
(mockFsExistsSync as Mock).mockReturnValue(true);
|
||||
const userSettings = {
|
||||
slashCommands: { disabled: ['auth', 'quit'] },
|
||||
};
|
||||
const workspaceSettings = {
|
||||
// Workspace overlaps with user and adds one entry. UNION de-dupes the
|
||||
// overlap and merges the new entry; it cannot remove user entries.
|
||||
slashCommands: { disabled: ['quit', 'clear'] },
|
||||
};
|
||||
|
||||
(fs.readFileSync as Mock).mockImplementation(
|
||||
(p: fs.PathOrFileDescriptor) => {
|
||||
if (p === USER_SETTINGS_PATH) return JSON.stringify(userSettings);
|
||||
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
|
||||
return JSON.stringify(workspaceSettings);
|
||||
return '{}';
|
||||
},
|
||||
);
|
||||
|
||||
const settings = loadSettings(MOCK_WORKSPACE_DIR);
|
||||
const disabled = settings.merged.slashCommands?.disabled ?? [];
|
||||
expect(disabled).toEqual(
|
||||
expect.arrayContaining(['auth', 'quit', 'clear']),
|
||||
);
|
||||
expect(disabled).toHaveLength(3);
|
||||
});
|
||||
|
||||
it('should merge all settings files with the correct precedence', () => {
|
||||
(mockFsExistsSync as Mock).mockReturnValue(true);
|
||||
const systemDefaultsContent = {
|
||||
|
|
|
|||
|
|
@ -1151,6 +1151,36 @@ const SETTINGS_SCHEMA = {
|
|||
},
|
||||
},
|
||||
|
||||
slashCommands: {
|
||||
type: 'object',
|
||||
label: 'Slash Commands',
|
||||
category: 'Advanced',
|
||||
requiresRestart: true,
|
||||
default: {},
|
||||
description:
|
||||
'Configuration for slash commands exposed by the CLI. Useful for ' +
|
||||
'locking down the command surface in multi-tenant or enterprise ' +
|
||||
'deployments.',
|
||||
showInDialog: false,
|
||||
properties: {
|
||||
disabled: {
|
||||
type: 'array',
|
||||
label: 'Disabled Slash Commands',
|
||||
category: 'Advanced',
|
||||
requiresRestart: true,
|
||||
default: undefined as string[] | undefined,
|
||||
description:
|
||||
'Slash command names to hide and refuse to execute. Matched ' +
|
||||
'case-insensitively against the final command name (for extension ' +
|
||||
'commands this is the disambiguated form, e.g. "myext.deploy"). ' +
|
||||
'Merged as a union across settings scopes, so workspace settings ' +
|
||||
'can add to but not remove entries defined in system/user settings.',
|
||||
showInDialog: false,
|
||||
mergeStrategy: MergeStrategy.UNION,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
tools: {
|
||||
type: 'object',
|
||||
label: 'Tools',
|
||||
|
|
|
|||
|
|
@ -607,6 +607,7 @@ describe('gemini.tsx main function kitty protocol', () => {
|
|||
resume: undefined,
|
||||
coreTools: undefined,
|
||||
excludeTools: undefined,
|
||||
disabledSlashCommands: undefined,
|
||||
authType: undefined,
|
||||
maxSessionTurns: undefined,
|
||||
experimentalLsp: undefined,
|
||||
|
|
|
|||
|
|
@ -151,6 +151,7 @@ describe('runNonInteractive', () => {
|
|||
setRegisterCallback: vi.fn(),
|
||||
getRunning: vi.fn().mockReturnValue([]),
|
||||
}),
|
||||
getDisabledSlashCommands: vi.fn().mockReturnValue([]),
|
||||
} as unknown as Config;
|
||||
|
||||
mockSettings = {
|
||||
|
|
|
|||
|
|
@ -36,6 +36,7 @@ describe('handleSlashCommand', () => {
|
|||
getFolderTrustFeature: vi.fn().mockReturnValue(false),
|
||||
getFolderTrust: vi.fn().mockReturnValue(false),
|
||||
getProjectRoot: vi.fn().mockReturnValue('/test/project'),
|
||||
getDisabledSlashCommands: vi.fn().mockReturnValue([]),
|
||||
storage: {},
|
||||
} as unknown as Config;
|
||||
|
||||
|
|
@ -122,6 +123,68 @@ describe('handleSlashCommand', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('should return unsupported (not no_command) for a disabled command so it is not forwarded to the model', async () => {
|
||||
const mockInitCommand = {
|
||||
name: 'init',
|
||||
description: 'Initialize project',
|
||||
kind: CommandKind.BUILT_IN,
|
||||
action: vi.fn(),
|
||||
};
|
||||
mockGetCommands.mockReturnValue([mockInitCommand]);
|
||||
vi.mocked(mockConfig.getDisabledSlashCommands).mockReturnValue(['init']);
|
||||
|
||||
const result = await handleSlashCommand(
|
||||
'/init',
|
||||
abortController,
|
||||
mockConfig,
|
||||
mockSettings,
|
||||
['init'], // Would normally be allowed; denylist must still block it.
|
||||
);
|
||||
|
||||
expect(result.type).toBe('unsupported');
|
||||
if (result.type === 'unsupported') {
|
||||
expect(result.reason).toContain('/init');
|
||||
expect(result.reason).toContain('disabled');
|
||||
}
|
||||
expect(mockInitCommand.action).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should match disabled names case-insensitively', async () => {
|
||||
const mockInitCommand = {
|
||||
name: 'init',
|
||||
description: 'Initialize project',
|
||||
kind: CommandKind.BUILT_IN,
|
||||
action: vi.fn(),
|
||||
};
|
||||
mockGetCommands.mockReturnValue([mockInitCommand]);
|
||||
vi.mocked(mockConfig.getDisabledSlashCommands).mockReturnValue(['INIT']);
|
||||
|
||||
const result = await handleSlashCommand(
|
||||
'/init',
|
||||
abortController,
|
||||
mockConfig,
|
||||
mockSettings,
|
||||
['init'],
|
||||
);
|
||||
|
||||
expect(result.type).toBe('unsupported');
|
||||
expect(mockInitCommand.action).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should still return no_command for truly unknown slash commands even when a denylist is set', async () => {
|
||||
mockGetCommands.mockReturnValue([]);
|
||||
vi.mocked(mockConfig.getDisabledSlashCommands).mockReturnValue(['help']);
|
||||
|
||||
const result = await handleSlashCommand(
|
||||
'/does-not-exist',
|
||||
abortController,
|
||||
mockConfig,
|
||||
mockSettings,
|
||||
);
|
||||
|
||||
expect(result.type).toBe('no_command');
|
||||
});
|
||||
|
||||
it('should execute allowed built-in commands', async () => {
|
||||
const mockInitCommand = {
|
||||
name: 'init',
|
||||
|
|
|
|||
|
|
@ -254,8 +254,19 @@ export const handleSlashCommand = async (
|
|||
: 'non_interactive';
|
||||
|
||||
const allowedBuiltinSet = new Set(allowedBuiltinCommandNames ?? []);
|
||||
const disabledSlashCommandsRaw = config.getDisabledSlashCommands();
|
||||
const disabledNameSet = new Set<string>();
|
||||
for (const name of disabledSlashCommandsRaw) {
|
||||
const trimmed = name.trim();
|
||||
if (trimmed) disabledNameSet.add(trimmed.toLowerCase());
|
||||
}
|
||||
const isDisabled = (cmd: { name: string }) =>
|
||||
disabledNameSet.has(cmd.name.toLowerCase());
|
||||
|
||||
// Load all commands to check if the command exists but is not allowed
|
||||
// Load the full command set (unfiltered by the denylist) so that the
|
||||
// fallback existence check below can distinguish a disabled command from a
|
||||
// truly unknown one. Without this, a disabled command would fall through to
|
||||
// `no_command` and be forwarded to the model as plain prompt text.
|
||||
const allLoaders = [
|
||||
new BuiltinCommandLoader(config),
|
||||
new BundledSkillLoader(config),
|
||||
|
|
@ -270,7 +281,7 @@ export const handleSlashCommand = async (
|
|||
const filteredCommands = filterCommandsForNonInteractive(
|
||||
allCommands,
|
||||
allowedBuiltinSet,
|
||||
);
|
||||
).filter((cmd) => !isDisabled(cmd));
|
||||
|
||||
// First, try to parse with filtered commands
|
||||
const { commandToExecute, args } = parseSlashCommand(
|
||||
|
|
@ -286,6 +297,16 @@ export const handleSlashCommand = async (
|
|||
);
|
||||
|
||||
if (knownCommand) {
|
||||
if (isDisabled(knownCommand)) {
|
||||
return {
|
||||
type: 'unsupported',
|
||||
reason: t(
|
||||
'The command "/{{command}}" is disabled by the current configuration.',
|
||||
{ command: knownCommand.name },
|
||||
),
|
||||
originalType: 'filtered_command',
|
||||
};
|
||||
}
|
||||
// Command exists but is not allowed in non-interactive mode
|
||||
return {
|
||||
type: 'unsupported',
|
||||
|
|
@ -380,7 +401,14 @@ export const getAvailableCommands = async (
|
|||
]
|
||||
: [new BundledSkillLoader(config), new FileCommandLoader(config)];
|
||||
|
||||
const commandService = await CommandService.create(loaders, abortSignal);
|
||||
const disabledSlashCommands = config.getDisabledSlashCommands();
|
||||
const commandService = await CommandService.create(
|
||||
loaders,
|
||||
abortSignal,
|
||||
disabledSlashCommands.length > 0
|
||||
? new Set(disabledSlashCommands)
|
||||
: undefined,
|
||||
);
|
||||
const commands = commandService.getCommands();
|
||||
const filteredCommands = filterCommandsForNonInteractive(
|
||||
commands,
|
||||
|
|
|
|||
|
|
@ -310,6 +310,80 @@ describe('CommandService', () => {
|
|||
expect(deployExtension?.description).toBe('[gcp] Deploy to Google Cloud');
|
||||
});
|
||||
|
||||
describe('disabledNames filtering', () => {
|
||||
it('should omit commands whose names are in the disabled set', async () => {
|
||||
const loader = new MockCommandLoader([
|
||||
mockCommandA,
|
||||
mockCommandB,
|
||||
mockCommandC,
|
||||
]);
|
||||
const service = await CommandService.create(
|
||||
[loader],
|
||||
new AbortController().signal,
|
||||
new Set(['command-b']),
|
||||
);
|
||||
const names = service.getCommands().map((cmd) => cmd.name);
|
||||
expect(names).toEqual(expect.arrayContaining(['command-a', 'command-c']));
|
||||
expect(names).not.toContain('command-b');
|
||||
});
|
||||
|
||||
it('should match disabled names case-insensitively', async () => {
|
||||
const loader = new MockCommandLoader([mockCommandA, mockCommandB]);
|
||||
const service = await CommandService.create(
|
||||
[loader],
|
||||
new AbortController().signal,
|
||||
new Set(['COMMAND-A']),
|
||||
);
|
||||
const names = service.getCommands().map((cmd) => cmd.name);
|
||||
expect(names).toEqual(['command-b']);
|
||||
});
|
||||
|
||||
it('should ignore empty entries and whitespace in the disabled set', async () => {
|
||||
const loader = new MockCommandLoader([mockCommandA, mockCommandB]);
|
||||
const service = await CommandService.create(
|
||||
[loader],
|
||||
new AbortController().signal,
|
||||
new Set(['', ' ', ' command-a ']),
|
||||
);
|
||||
const names = service.getCommands().map((cmd) => cmd.name);
|
||||
expect(names).toEqual(['command-b']);
|
||||
});
|
||||
|
||||
it('should be a no-op when disabledNames is undefined or empty', async () => {
|
||||
const loader = new MockCommandLoader([mockCommandA, mockCommandB]);
|
||||
const undefinedResult = await CommandService.create(
|
||||
[loader],
|
||||
new AbortController().signal,
|
||||
);
|
||||
expect(undefinedResult.getCommands()).toHaveLength(2);
|
||||
|
||||
const emptyResult = await CommandService.create(
|
||||
[new MockCommandLoader([mockCommandA, mockCommandB])],
|
||||
new AbortController().signal,
|
||||
new Set<string>(),
|
||||
);
|
||||
expect(emptyResult.getCommands()).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('should disable extension commands by their renamed (final) name', async () => {
|
||||
const builtin = createMockCommand('deploy', CommandKind.BUILT_IN);
|
||||
const extension = {
|
||||
...createMockCommand('deploy', CommandKind.FILE),
|
||||
extensionName: 'firebase',
|
||||
description: '[firebase] Deploy to Firebase',
|
||||
};
|
||||
const loader = new MockCommandLoader([builtin, extension]);
|
||||
const service = await CommandService.create(
|
||||
[loader],
|
||||
new AbortController().signal,
|
||||
new Set(['firebase.deploy']),
|
||||
);
|
||||
const names = service.getCommands().map((cmd) => cmd.name);
|
||||
// Built-in /deploy remains; the renamed extension command is gone.
|
||||
expect(names).toEqual(['deploy']);
|
||||
});
|
||||
});
|
||||
|
||||
it('should handle multiple secondary conflicts with incrementing suffixes', async () => {
|
||||
// User has /deploy, /gcp.deploy, and /gcp.deploy1
|
||||
const userCommand1 = createMockCommand('deploy', CommandKind.FILE);
|
||||
|
|
|
|||
|
|
@ -33,8 +33,9 @@ export class CommandService {
|
|||
*
|
||||
* This factory method orchestrates the entire command loading process. It
|
||||
* runs all provided loaders in parallel, aggregates their results, handles
|
||||
* name conflicts for extension commands by renaming them, and then returns a
|
||||
* fully constructed `CommandService` instance.
|
||||
* name conflicts for extension commands by renaming them, optionally filters
|
||||
* out disabled commands, and then returns a fully constructed
|
||||
* `CommandService` instance.
|
||||
*
|
||||
* Conflict resolution:
|
||||
* - Extension commands that conflict with existing commands are renamed to
|
||||
|
|
@ -45,11 +46,16 @@ export class CommandService {
|
|||
* @param loaders An array of objects that conform to the `ICommandLoader`
|
||||
* interface. Built-in commands should come first, followed by FileCommandLoader.
|
||||
* @param signal An AbortSignal to cancel the loading process.
|
||||
* @param disabledNames Optional set of command names to exclude. Matched
|
||||
* case-insensitively against the final (post-rename) command name. Intended
|
||||
* for settings- or flag-driven denylists that gate the CLI surface (see
|
||||
* `slashCommands.disabled` and `--disabled-slash-commands`).
|
||||
* @returns A promise that resolves to a new, fully initialized `CommandService` instance.
|
||||
*/
|
||||
static async create(
|
||||
loaders: ICommandLoader[],
|
||||
signal: AbortSignal,
|
||||
disabledNames?: ReadonlySet<string>,
|
||||
): Promise<CommandService> {
|
||||
const results = await Promise.allSettled(
|
||||
loaders.map((loader) => loader.loadCommands(signal)),
|
||||
|
|
@ -88,6 +94,21 @@ export class CommandService {
|
|||
});
|
||||
}
|
||||
|
||||
if (disabledNames && disabledNames.size > 0) {
|
||||
const normalizedDisabled = new Set<string>();
|
||||
for (const entry of disabledNames) {
|
||||
const trimmed = entry.trim();
|
||||
if (trimmed) normalizedDisabled.add(trimmed.toLowerCase());
|
||||
}
|
||||
if (normalizedDisabled.size > 0) {
|
||||
for (const name of Array.from(commandMap.keys())) {
|
||||
if (normalizedDisabled.has(name.toLowerCase())) {
|
||||
commandMap.delete(name);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const finalCommands = Object.freeze(Array.from(commandMap.values()));
|
||||
return new CommandService(finalCommands);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -353,9 +353,11 @@ export const useSlashCommandProcessor = (
|
|||
new BundledSkillLoader(config),
|
||||
new FileCommandLoader(config),
|
||||
];
|
||||
const disabled = config?.getDisabledSlashCommands() ?? [];
|
||||
const commandService = await CommandService.create(
|
||||
loaders,
|
||||
controller.signal,
|
||||
disabled.length > 0 ? new Set(disabled) : undefined,
|
||||
);
|
||||
// Avoid overwriting newer results from a subsequent effect run
|
||||
if (!controller.signal.aborted) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue