refactor(claudecode): centralize .claude/settings.json permissions ownership in a gateway

src/features/claudecode-settings-gateway.test.ts

@@ -0,0 +1,138 @@
+import { describe, expect, it } from "vitest";
+
+import { createMockLogger } from "../test-utils/mock-logger.js";
+import type { ClaudeSettingsJson } from "../types/claude-settings.js";
+import {
+  applyIgnoreReadDenies,
+  applyPermissions,
+  buildReadDenyEntry,
+  isReadDenyEntry,
+} from "./claudecode-settings-gateway.js";
+
+// The permissions feature parses "Bash(npm *)" into its tool name; the gateway
+// is agnostic to the format and takes this extractor as a parameter.
+const toolNameOf = (entry: string): string => {
+  const parenIndex = entry.indexOf("(");
+  return parenIndex === -1 ? entry : entry.slice(0, parenIndex);
+};
+
+describe("isReadDenyEntry", () => {
+  it("recognizes Read(...) entries", () => {
+    expect(isReadDenyEntry("Read(.env)")).toBe(true);
+    expect(isReadDenyEntry("Read(*.log)")).toBe(true);
+  });
+
+  it("rejects non-Read and malformed entries", () => {
+    expect(isReadDenyEntry("Write(secret.txt)")).toBe(false);
+    expect(isReadDenyEntry("Read(unterminated")).toBe(false);
+    expect(isReadDenyEntry("Bash")).toBe(false);
+  });
+});
+
+describe("buildReadDenyEntry", () => {
+  it("wraps a pattern into a Read deny entry", () => {
+    expect(buildReadDenyEntry("*.log")).toBe("Read(*.log)");
+  });
+});
+
+describe("applyIgnoreReadDenies", () => {
+  it("preserves non-Read deny entries while replacing the Read set", () => {
+    const settings: ClaudeSettingsJson = {
+      permissions: { deny: ["Write(secret.txt)", "Read(old.log)"] },
+    };
+
+    const result = applyIgnoreReadDenies({
+      settings,
+      readDenies: ["Read(*.log)", "Read(node_modules/**)"],
+    });
+
+    expect(result.permissions?.deny).toEqual([
+      "Read(*.log)",
+      "Read(node_modules/**)",
+      "Write(secret.txt)",
+    ]);
+  });
+
+  it("leaves allow/ask untouched and other top-level keys intact", () => {
+    const settings: ClaudeSettingsJson = {
+      permissions: { allow: ["Bash(ls)"], ask: ["Bash(rm *)"], deny: ["Read(a)"] },
+      hooks: { PreToolUse: [] },
+    };
+
+    const result = applyIgnoreReadDenies({ settings, readDenies: ["Read(b)"] });
+
+    expect(result.permissions?.allow).toEqual(["Bash(ls)"]);
+    expect(result.permissions?.ask).toEqual(["Bash(rm *)"]);
+    expect(result.permissions?.deny).toEqual(["Read(b)"]);
+    expect(result.hooks).toEqual({ PreToolUse: [] });
+  });
+
+  it("deduplicates and sorts", () => {
+    const result = applyIgnoreReadDenies({
+      settings: { permissions: { deny: ["Read(z)"] } },
+      readDenies: ["Read(b)", "Read(a)", "Read(b)"],
+    });
+
+    expect(result.permissions?.deny).toEqual(["Read(a)", "Read(b)"]);
+  });
+});
+
+describe("applyPermissions", () => {
+  it("keeps entries for unmanaged tools and replaces managed ones", () => {
+    const settings: ClaudeSettingsJson = {
+      permissions: { deny: ["Read(.env)", "Bash(dangerous *)"] },
+    };
+
+    const result = applyPermissions({
+      settings,
+      managedToolNames: new Set(["Bash"]),
+      toolNameOf,
+      allow: [],
+      ask: [],
+      deny: ["Bash(rm *)"],
+    });
+
+    // Read (unmanaged) preserved; old Bash replaced by the new Bash rule.
+    expect(result.permissions?.deny).toEqual(["Bash(rm *)", "Read(.env)"]);
+  });
+
+  it("overwrites ignore-derived Read denies when Read is managed and warns", () => {
+    const logger = createMockLogger();
+    const settings: ClaudeSettingsJson = {
+      permissions: { deny: ["Read(.env)", "Read(*.secret)"] },
+    };
+
+    const result = applyPermissions({
+      settings,
+      managedToolNames: new Set(["Read"]),
+      toolNameOf,
+      allow: ["Read(src/**)"],
+      ask: [],
+      deny: [],
+      logger,
+    });
+
+    expect(result.permissions?.deny).toBeUndefined();
+    expect(result.permissions?.allow).toEqual(["Read(src/**)"]);
+    expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("manages 'Read' tool"));
+    expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining("2 existing Read deny"));
+    // The warning no longer speculates about the ignore feature by name.
+    expect(logger.warn).not.toHaveBeenCalledWith(expect.stringContaining("ignore"));
+  });
+
+  it("does not warn when Read is not managed", () => {
+    const logger = createMockLogger();
+
+    applyPermissions({
+      settings: { permissions: { deny: ["Read(.env)"] } },
+      managedToolNames: new Set(["Bash"]),
+      toolNameOf,
+      allow: [],
+      ask: [],
+      deny: ["Bash(rm *)"],
+      logger,
+    });
+
+    expect(logger.warn).not.toHaveBeenCalled();
+  });
+});

src/features/claudecode-settings-gateway.ts

@@ -0,0 +1,109 @@
+import { uniq } from "es-toolkit";
+
+import type { ClaudeSettingsJson } from "../types/claude-settings.js";
+import type { Logger } from "../utils/logger.js";
+
+/**
+ * Single owner of the `.claude/settings.json` `permissions` block, which both
+ * `ignore` (writes `Read(...)` into `permissions.deny`) and `permissions`
+ * (writes the whole `allow`/`ask`/`deny`) read-modify-write. The entry format,
+ * the merge, and the cross-feature ownership rule (permissions' explicit `Read`
+ * rules win over ignore-derived `Read` denies) used to be duplicated across both
+ * feature files; they live here once so each feature just states its intent and
+ * never reasons about the other's existence.
+ */
+
+const READ_TOOL_NAME = "Read";
+
+export const isReadDenyEntry = (entry: string): boolean =>
+  entry.startsWith(`${READ_TOOL_NAME}(`) && entry.endsWith(")");
+
+export const buildReadDenyEntry = (pattern: string): string => `${READ_TOOL_NAME}(${pattern})`;
+
+const parsePermissionsBlock = (
+  settings: ClaudeSettingsJson,
+): { allow: string[]; ask: string[]; deny: string[] } => {
+  const permissions = settings.permissions ?? {};
+  return {
+    allow: permissions.allow ?? [],
+    ask: permissions.ask ?? [],
+    deny: permissions.deny ?? [],
+  };
+};
+
+// Empty arrays are omitted so the file never carries an empty allow/ask/deny key.
+// Other top-level keys (e.g. `hooks`) and other keys under `permissions` are kept.
+const withPermissions = (
+  settings: ClaudeSettingsJson,
+  next: { allow: string[]; ask: string[]; deny: string[] },
+): ClaudeSettingsJson => {
+  const permissions: Record<string, unknown> = { ...settings.permissions };
+  const assign = (key: "allow" | "ask" | "deny", values: string[]): void => {
+    if (values.length > 0) {
+      permissions[key] = values;
+    } else {
+      delete permissions[key];
+    }
+  };
+  assign("allow", next.allow);
+  assign("ask", next.ask);
+  assign("deny", next.deny);
+  return { ...settings, permissions };
+};
+
+// Non-`Read` deny entries belong to the permissions feature and are preserved;
+// `Read(...)` denies are replaced wholesale since the ignore source owns them.
+export const applyIgnoreReadDenies = (params: {
+  settings: ClaudeSettingsJson;
+  readDenies: string[];
+}): ClaudeSettingsJson => {
+  const { settings, readDenies } = params;
+  const current = parsePermissionsBlock(settings);
+  const preservedDeny = current.deny.filter(
+    (entry) => !isReadDenyEntry(entry) || readDenies.includes(entry),
+  );
+  return withPermissions(settings, {
+    allow: current.allow,
+    ask: current.ask,
+    deny: uniq([...preservedDeny, ...readDenies].toSorted()),
+  });
+};
+
+// Entries for managed tools are replaced; entries for unmanaged tools are kept.
+// When `Read` is managed, permissions' rules win over ignore-derived `Read(...)`
+// denies — those are overwritten, and the overwrite is warned about if a logger
+// is given.
+export const applyPermissions = (params: {
+  settings: ClaudeSettingsJson;
+  managedToolNames: ReadonlySet<string>;
+  toolNameOf: (entry: string) => string;
+  allow: string[];
+  ask: string[];
+  deny: string[];
+  logger?: Logger | undefined;
+}): ClaudeSettingsJson => {
+  const { settings, managedToolNames, toolNameOf, allow, ask, deny, logger } = params;
+  const current = parsePermissionsBlock(settings);
+
+  const keepUnmanaged = (entries: string[]): string[] =>
+    entries.filter((entry) => !managedToolNames.has(toolNameOf(entry)));
+
+  if (logger && managedToolNames.has(READ_TOOL_NAME)) {
+    const overwrittenReadDenies = current.deny.filter(
+      (entry) => toolNameOf(entry) === READ_TOOL_NAME,
+    );
+    if (overwrittenReadDenies.length > 0) {
+      logger.warn(
+        `Permissions feature manages '${READ_TOOL_NAME}' tool and will overwrite ` +
+          `${overwrittenReadDenies.length} existing ${READ_TOOL_NAME} deny entries. ` +
+          `Permissions take precedence.`,
+      );
+    }
+  }
+
+  return withPermissions(settings, {
+    allow: uniq([...keepUnmanaged(current.allow), ...allow].toSorted()),
+    ask: uniq([...keepUnmanaged(current.ask), ...ask].toSorted()),
+    deny: uniq([...keepUnmanaged(current.deny), ...deny].toSorted()),
+  });
+};

src/features/ignore/claudecode-ignore.ts

@@ -1,6 +1,5 @@
 import { join } from "node:path";
 
-import { uniq } from "es-toolkit";
 import { z } from "zod/mini";
 
 import {
@@ -11,6 +10,11 @@ import {
 import type { ClaudeSettingsJson } from "../../types/claude-settings.js";
 import { FeatureOptions } from "../../types/features.js";
 import { fileExists, readFileContent } from "../../utils/file.js";
+import {
+  applyIgnoreReadDenies,
+  buildReadDenyEntry,
+  isReadDenyEntry,
+} from "../claudecode-settings-gateway.js";
 import { RulesyncIgnore } from "./rulesync-ignore.js";
 import {
   ToolIgnore,
@@ -104,7 +108,7 @@ export class ClaudecodeIgnore extends ToolIgnore {
     const rulesyncPatterns = this.patterns
       .map((pattern) => {
         // Remove "Read(" prefix and ")" suffix if present
-        if (pattern.startsWith("Read(") && pattern.endsWith(")")) {
+        if (isReadDenyEntry(pattern)) {
           return pattern.slice(5, -1);
         }
         return pattern;
@@ -133,30 +137,20 @@ export class ClaudecodeIgnore extends ToolIgnore {
       .split(/\r?\n|\r/)
       .map((line: string) => line.trim())
       .filter((line) => line.length > 0 && !line.startsWith("#"));
-    const deniedValues = patterns.map((pattern) => `Read(${pattern})`);
+    const deniedValues = patterns.map((pattern) => buildReadDenyEntry(pattern));
 
     const paths = this.getSettablePaths({ options });
     const filePath = join(outputRoot, paths.relativeDirPath, paths.relativeFilePath);
     const exists = await fileExists(filePath);
     const existingFileContent = exists ? await readFileContent(filePath) : "{}";
     const existingJsonValue: ClaudeSettingsJson = JSON.parse(existingFileContent);
-    const existingDenies = existingJsonValue.permissions?.deny ?? [];
-    const preservedDenies = existingDenies.filter((deny) => {
-      const isReadPattern = deny.startsWith("Read(") && deny.endsWith(")");
-      if (isReadPattern) {
-        return deniedValues.includes(deny);
-      }
-
-      return true;
-    });
 
-    const jsonValue: ClaudeSettingsJson = {
-      ...existingJsonValue,
-      permissions: {
-        ...existingJsonValue.permissions,
-        deny: uniq([...preservedDenies, ...deniedValues].toSorted()),
-      },
-    };
+    // The gateway owns the `permissions.deny` merge shared with the permissions
+    // feature; here we only state the intent (deny these Read patterns).
+    const jsonValue = applyIgnoreReadDenies({
+      settings: existingJsonValue,
+      readDenies: deniedValues,
+    });
 
     return new ClaudecodeIgnore({
       outputRoot,