From 8454fff5e7d611230aa15560b31433d60dfb9877 Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Tue, 7 Jul 2026 04:06:34 -0700 Subject: [PATCH] feat(permissions): map Kiro grep/glob categories to toolsSettings rulesync already carries canonical grep/glob categories, but the Kiro translator dropped them with an "unsupported category" warning even though Kiro documents toolsSettings.grep/glob.{allowedPaths,deniedPaths}. Map grep -> toolsSettings.grep and glob -> toolsSettings.glob (mirroring the read mapping), emitting each table only when a rule is present so existing configs do not gain empty tables, and round-trip both on import. Refactored the path-category handling into shared helpers to keep the translator under the complexity limit. This is item 1 of #2132 (the straightforward, override-independent fix). The kiro-scoped override namespace for aws/shell-auto-trust/web-domain/ MCP keys (item 2) is left as a follow-up: it carries several open design questions and touches the shared .kiro/agents/default.json round-trip. Source: https://kiro.dev/docs/cli/custom-agents/configuration-reference/ Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/reference/file-formats.md | 2 + skills/rulesync/file-formats.md | 2 + .../permissions/kiro-permissions.test.ts | 60 ++++++++ src/features/permissions/kiro-permissions.ts | 141 +++++++++++------- 4 files changed, 152 insertions(+), 53 deletions(-) diff --git a/docs/reference/file-formats.md b/docs/reference/file-formats.md index 80deeafa1..98f4923e6 100644 --- a/docs/reference/file-formats.md +++ b/docs/reference/file-formats.md @@ -909,6 +909,8 @@ For Kiro, this generates tool permission settings in `.kiro/agents/default.json` - `bash` maps to `toolsSettings.shell.allowedCommands` / `toolsSettings.shell.deniedCommands` - `read` maps to `toolsSettings.read.allowedPaths` / `toolsSettings.read.deniedPaths` - `edit` / `write` map to `toolsSettings.write.allowedPaths` / `toolsSettings.write.deniedPaths` +- `grep` maps to `toolsSettings.grep.allowedPaths` / `toolsSettings.grep.deniedPaths` +- `glob` maps to `toolsSettings.glob.allowedPaths` / `toolsSettings.glob.deniedPaths` (both emitted only when a rule is present, so existing configs do not gain empty tables) - `webfetch` / `websearch` with pattern `*` map to `allowedTools` entries (`web_fetch` / `web_search`) - `ask` rules are skipped with a warning (Kiro config does not support explicit ask entries) diff --git a/skills/rulesync/file-formats.md b/skills/rulesync/file-formats.md index 80deeafa1..98f4923e6 100644 --- a/skills/rulesync/file-formats.md +++ b/skills/rulesync/file-formats.md @@ -909,6 +909,8 @@ For Kiro, this generates tool permission settings in `.kiro/agents/default.json` - `bash` maps to `toolsSettings.shell.allowedCommands` / `toolsSettings.shell.deniedCommands` - `read` maps to `toolsSettings.read.allowedPaths` / `toolsSettings.read.deniedPaths` - `edit` / `write` map to `toolsSettings.write.allowedPaths` / `toolsSettings.write.deniedPaths` +- `grep` maps to `toolsSettings.grep.allowedPaths` / `toolsSettings.grep.deniedPaths` +- `glob` maps to `toolsSettings.glob.allowedPaths` / `toolsSettings.glob.deniedPaths` (both emitted only when a rule is present, so existing configs do not gain empty tables) - `webfetch` / `websearch` with pattern `*` map to `allowedTools` entries (`web_fetch` / `web_search`) - `ask` rules are skipped with a warning (Kiro config does not support explicit ask entries) diff --git a/src/features/permissions/kiro-permissions.test.ts b/src/features/permissions/kiro-permissions.test.ts index bf463e624..7d5840404 100644 --- a/src/features/permissions/kiro-permissions.test.ts +++ b/src/features/permissions/kiro-permissions.test.ts @@ -86,6 +86,66 @@ describe("KiroPermissions", () => { expect(json.permission.webfetch?.["*"]).toBe("allow"); }); + it("should map grep/glob categories to Kiro toolsSettings", async () => { + const rulesyncPermissions = new RulesyncPermissions({ + outputRoot: testDir, + relativeDirPath: ".rulesync", + relativeFilePath: "permissions.json", + fileContent: JSON.stringify({ + permission: { + grep: { "src/**": "allow", "secrets/**": "deny" }, + glob: { "**/*.ts": "allow" }, + }, + }), + }); + + const kiroPermissions = await KiroPermissions.fromRulesyncPermissions({ + outputRoot: testDir, + rulesyncPermissions, + }); + + const content = JSON.parse(kiroPermissions.getFileContent()); + expect(content.toolsSettings.grep.allowedPaths).toEqual(["src/**"]); + expect(content.toolsSettings.grep.deniedPaths).toEqual(["secrets/**"]); + expect(content.toolsSettings.glob.allowedPaths).toEqual(["**/*.ts"]); + }); + + it("should not emit empty grep/glob tables when unused", async () => { + const rulesyncPermissions = new RulesyncPermissions({ + outputRoot: testDir, + relativeDirPath: ".rulesync", + relativeFilePath: "permissions.json", + fileContent: JSON.stringify({ permission: { bash: { "git *": "allow" } } }), + }); + + const kiroPermissions = await KiroPermissions.fromRulesyncPermissions({ + outputRoot: testDir, + rulesyncPermissions, + }); + + const content = JSON.parse(kiroPermissions.getFileContent()); + expect(content.toolsSettings.grep).toBeUndefined(); + expect(content.toolsSettings.glob).toBeUndefined(); + }); + + it("should round-trip grep/glob from Kiro toolsSettings", () => { + const kiroPermissions = new KiroPermissions({ + outputRoot: testDir, + relativeDirPath: join(".kiro", "agents"), + relativeFilePath: "default.json", + fileContent: JSON.stringify({ + toolsSettings: { + grep: { allowedPaths: ["src/**"], deniedPaths: ["secrets/**"] }, + glob: { allowedPaths: ["**/*.ts"], deniedPaths: [] }, + }, + }), + }); + + const json = kiroPermissions.toRulesyncPermissions().getJson(); + expect(json.permission.grep).toEqual({ "src/**": "allow", "secrets/**": "deny" }); + expect(json.permission.glob).toEqual({ "**/*.ts": "allow" }); + }); + it("should load existing .kiro/agents/default.json", async () => { const kiroDir = join(testDir, ".kiro", "agents"); await ensureDir(kiroDir); diff --git a/src/features/permissions/kiro-permissions.ts b/src/features/permissions/kiro-permissions.ts index 787519dcb..3c7de768b 100644 --- a/src/features/permissions/kiro-permissions.ts +++ b/src/features/permissions/kiro-permissions.ts @@ -4,7 +4,7 @@ import { z } from "zod/mini"; import { KIRO_AGENTS_DIR_PATH, KIRO_HOOKS_FILE_NAME } from "../../constants/kiro-paths.js"; import type { ValidationResult } from "../../types/ai-file.js"; -import type { PermissionsConfig } from "../../types/permissions.js"; +import type { PermissionAction, PermissionsConfig } from "../../types/permissions.js"; import { formatError } from "../../utils/error.js"; import { readFileContentOrNull } from "../../utils/file.js"; import { RulesyncPermissions } from "./rulesync-permissions.js"; @@ -95,31 +95,22 @@ export class KiroPermissions extends ToolPermissions { const permission: PermissionsConfig["permission"] = {}; const toolsSettings = parsed.toolsSettings ?? {}; - const shellSettings = asRecord(toolsSettings.shell); - const shellAllow = asStringArray(shellSettings.allowedCommands); - const shellDeny = asStringArray(shellSettings.deniedCommands); - if (shellAllow.length > 0 || shellDeny.length > 0) { - permission.bash = {}; - for (const pattern of shellAllow) permission.bash[pattern] = "allow"; - for (const pattern of shellDeny) permission.bash[pattern] = "deny"; - } - - const readSettings = asRecord(toolsSettings.read); - const readAllow = asStringArray(readSettings.allowedPaths); - const readDeny = asStringArray(readSettings.deniedPaths); - if (readAllow.length > 0 || readDeny.length > 0) { - permission.read = {}; - for (const pattern of readAllow) permission.read[pattern] = "allow"; - for (const pattern of readDeny) permission.read[pattern] = "deny"; - } + const shellRules = rulesFromArrays( + asRecord(toolsSettings.shell), + "allowedCommands", + "deniedCommands", + ); + if (Object.keys(shellRules).length > 0) permission.bash = shellRules; - const writeSettings = asRecord(toolsSettings.write); - const writeAllow = asStringArray(writeSettings.allowedPaths); - const writeDeny = asStringArray(writeSettings.deniedPaths); - if (writeAllow.length > 0 || writeDeny.length > 0) { - permission.write = {}; - for (const pattern of writeAllow) permission.write[pattern] = "allow"; - for (const pattern of writeDeny) permission.write[pattern] = "deny"; + // read/write/grep/glob all use `{ allowedPaths, deniedPaths }` under their + // own toolsSettings key, mapping 1:1 to the canonical category name. + for (const category of ["read", "write", "grep", "glob"] as const) { + const rules = rulesFromArrays( + asRecord(toolsSettings[category]), + "allowedPaths", + "deniedPaths", + ); + if (Object.keys(rules).length > 0) permission[category] = rules; } const allowedTools = new Set(parsed.allowedTools ?? []); @@ -166,18 +157,14 @@ function buildKiroPermissionsFromRulesync({ const nextAllowedTools = new Set(existing.allowedTools ?? []); const nextToolsSettings = { ...asRecord(existing.toolsSettings) }; - const shell: { allowedCommands: string[]; deniedCommands: string[] } = { - allowedCommands: [], - deniedCommands: [], - }; - const read: { allowedPaths: string[]; deniedPaths: string[] } = { - allowedPaths: [], - deniedPaths: [], - }; - const write: { allowedPaths: string[]; deniedPaths: string[] } = { - allowedPaths: [], - deniedPaths: [], + // Path/command categories map to a `{ : [], : [] }` table + // under a `toolsSettings` key. `edit` and `write` both fold into `write`. + const pathBuckets: Record = {}; + const pushPath = (key: string, action: PermissionAction, pattern: string): void => { + const bucket = (pathBuckets[key] ??= { allow: [], deny: [] }); + (action === "allow" ? bucket.allow : bucket.deny).push(pattern); }; + const shell = { allowedCommands: [] as string[], deniedCommands: [] as string[] }; for (const [category, rules] of Object.entries(config.permission)) { for (const [pattern, action] of Object.entries(rules)) { @@ -187,32 +174,30 @@ function buildKiroPermissionsFromRulesync({ } if (category === "bash") { (action === "allow" ? shell.allowedCommands : shell.deniedCommands).push(pattern); - } else if (category === "read") { - (action === "allow" ? read.allowedPaths : read.deniedPaths).push(pattern); + } else if (category === "read" || category === "grep" || category === "glob") { + pushPath(category, action, pattern); } else if (category === "edit" || category === "write") { - (action === "allow" ? write.allowedPaths : write.deniedPaths).push(pattern); + pushPath("write", action, pattern); } else if (category === "webfetch" || category === "websearch") { - if (pattern !== "*") { - logger?.warn( - `Kiro ${category} supports only wildcard (*) via allowedTools. Skipping rule: ${pattern}`, - ); - continue; - } - const toolName = category === "webfetch" ? "web_fetch" : "web_search"; - if (action === "allow") { - nextAllowedTools.add(toolName); - } else { - nextAllowedTools.delete(toolName); - } + applyKiroWebPermission({ category, pattern, action, nextAllowedTools, logger }); } else { logger?.warn(`Kiro permissions do not support category: ${category}. Skipping.`); } } } + // `shell`/`read`/`write` are always emitted (even empty) to match the prior + // behavior; `grep`/`glob` are only emitted when they carry a rule so existing + // configs do not gain empty tables. nextToolsSettings.shell = shell; - nextToolsSettings.read = read; - nextToolsSettings.write = write; + nextToolsSettings.read = pathTable(pathBuckets.read); + nextToolsSettings.write = pathTable(pathBuckets.write); + for (const key of ["grep", "glob"] as const) { + const bucket = pathBuckets[key]; + if (bucket && (bucket.allow.length > 0 || bucket.deny.length > 0)) { + nextToolsSettings[key] = pathTable(bucket); + } + } return { ...existing, @@ -221,6 +206,40 @@ function buildKiroPermissionsFromRulesync({ }; } +function pathTable(bucket: { allow: string[]; deny: string[] } | undefined): { + allowedPaths: string[]; + deniedPaths: string[]; +} { + return { allowedPaths: bucket?.allow ?? [], deniedPaths: bucket?.deny ?? [] }; +} + +function applyKiroWebPermission({ + category, + pattern, + action, + nextAllowedTools, + logger, +}: { + category: "webfetch" | "websearch"; + pattern: string; + action: PermissionAction; + nextAllowedTools: Set; + logger?: ToolPermissionsFromRulesyncPermissionsParams["logger"]; +}): void { + if (pattern !== "*") { + logger?.warn( + `Kiro ${category} supports only wildcard (*) via allowedTools. Skipping rule: ${pattern}`, + ); + return; + } + const toolName = category === "webfetch" ? "web_fetch" : "web_search"; + if (action === "allow") { + nextAllowedTools.add(toolName); + } else { + nextAllowedTools.delete(toolName); + } +} + function asRecord(value: unknown): Record { const result = UnknownRecordSchema.safeParse(value); return result.success ? result.data : {}; @@ -231,3 +250,19 @@ function asStringArray(value: unknown): string[] { ? value.filter((item): item is string => typeof item === "string") : []; } + +/** + * Build a canonical `{ pattern: action }` map from a Kiro tool settings record's + * allow/deny string arrays (e.g. `allowedPaths`/`deniedPaths` or + * `allowedCommands`/`deniedCommands`). + */ +function rulesFromArrays( + settings: Record, + allowKey: string, + denyKey: string, +): Record { + const rules: Record = {}; + for (const pattern of asStringArray(settings[allowKey])) rules[pattern] = "allow"; + for (const pattern of asStringArray(settings[denyKey])) rules[pattern] = "deny"; + return rules; +}