From f3e8b30b38a9c3106179f9ea4d9c921968a8f94c Mon Sep 17 00:00:00 2001 From: root Date: Mon, 11 May 2026 15:59:16 +0100 Subject: [PATCH 1/2] fix(codexcli): guard :project_roots overwrite, skip empty patterns, add negative test --- .../permissions/codexcli-permissions.test.ts | 31 +++++++++++++++++++ .../permissions/codexcli-permissions.ts | 16 +++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/features/permissions/codexcli-permissions.test.ts b/src/features/permissions/codexcli-permissions.test.ts index 11ba1e910..28b17e73f 100644 --- a/src/features/permissions/codexcli-permissions.test.ts +++ b/src/features/permissions/codexcli-permissions.test.ts @@ -119,6 +119,37 @@ default_permissions = "rulesync" expect(json.permission.webfetch?.["example.com"]).toBe("deny"); }); + it("should not set glob_scan_max_depth when project-root globs contain only single-level wildcards", async () => { + const logger = createMockLogger(); + const rulesyncPermissions = new RulesyncPermissions({ + outputRoot: testDir, + relativeDirPath: ".rulesync", + relativeFilePath: "permissions.json", + fileContent: JSON.stringify({ + permission: { + read: { + "src/*": "allow", + }, + write: { + "docs/*": "allow", + }, + }, + }), + }); + + const codexPermissions = await CodexcliPermissions.fromRulesyncPermissions({ + outputRoot: testDir, + rulesyncPermissions, + logger, + }); + + const fileContent = codexPermissions.getFileContent(); + expect(fileContent).toContain('[permissions.rulesync.filesystem.":project_roots"]'); + expect(fileContent).toContain('"src/*" = "read"'); + expect(fileContent).toContain('"docs/*" = "write"'); + expect(fileContent).not.toContain("glob_scan_max_depth"); + }); + it("should import nested Codex project root filesystem rules", () => { const codexPermissions = new CodexcliPermissions({ outputRoot: testDir, diff --git a/src/features/permissions/codexcli-permissions.ts b/src/features/permissions/codexcli-permissions.ts index 143fb6478..51d022f87 100644 --- a/src/features/permissions/codexcli-permissions.ts +++ b/src/features/permissions/codexcli-permissions.ts @@ -19,7 +19,7 @@ import { const RULESYNC_PROFILE_NAME = "rulesync"; const RULESYNC_BASH_RULES_FILE_NAME = "rulesync.rules"; const CODEX_PROJECT_ROOTS_KEY = ":project_roots"; -const CODEX_GLOB_SCAN_MAX_DEPTH = 8; +const CODEX_GLOB_SCAN_MAX_DEPTH = 8; // Matches Codex CLI default glob_scan_max_depth type CodexFilesystemAccess = "read" | "write" | "none"; type CodexFilesystemRuleTable = Record; @@ -180,6 +180,7 @@ function convertRulesyncToCodexProfile({ projectRootFilesystem, pattern, access: mapReadAction(action), + logger, }); } continue; @@ -192,6 +193,7 @@ function convertRulesyncToCodexProfile({ projectRootFilesystem, pattern, access: mapWriteAction(action), + logger, }); } continue; @@ -216,6 +218,11 @@ function convertRulesyncToCodexProfile({ } if (Object.keys(projectRootFilesystem).length > 0) { + if (typeof filesystem[CODEX_PROJECT_ROOTS_KEY] === "string") { + logger?.warn( + `":project_roots" is set as a direct filesystem access rule in the permissions, but it will be overwritten by project-root rules. Consider removing the direct ":project_roots" entry.`, + ); + } if (Object.keys(projectRootFilesystem).some((pattern) => pattern.includes("**"))) { filesystem.glob_scan_max_depth = CODEX_GLOB_SCAN_MAX_DEPTH; } @@ -275,12 +282,19 @@ function addFilesystemRule({ projectRootFilesystem, pattern, access, + logger, }: { filesystem: CodexFilesystem; projectRootFilesystem: CodexFilesystemRuleTable; pattern: string; access: CodexFilesystemAccess; + logger?: ToolPermissionsFromRulesyncPermissionsParams["logger"]; }): void { + if (pattern === "") { + logger?.warn("Skipping empty pattern in filesystem permissions."); + return; + } + if (canBeCodexFilesystemRoot(pattern)) { filesystem[pattern] = access; return; From 796e473769890c30ed942165dfef5372ac2f03a5 Mon Sep 17 00:00:00 2001 From: root Date: Tue, 12 May 2026 00:56:51 +0100 Subject: [PATCH 2/2] fix(codexcli): use constant in warning, trim empty patterns, add missing tests --- .../permissions/codexcli-permissions.test.ts | 55 +++++++++++++++++++ .../permissions/codexcli-permissions.ts | 4 +- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/features/permissions/codexcli-permissions.test.ts b/src/features/permissions/codexcli-permissions.test.ts index 28b17e73f..10fe8f8c4 100644 --- a/src/features/permissions/codexcli-permissions.test.ts +++ b/src/features/permissions/codexcli-permissions.test.ts @@ -179,6 +179,61 @@ glob_scan_max_depth = 8 expect(json.permission.edit?.["docs/**"]).toBe("allow"); }); + it("should warn when :project_roots is set as a direct string access rule", async () => { + const logger = createMockLogger(); + const rulesyncPermissions = new RulesyncPermissions({ + outputRoot: testDir, + relativeDirPath: ".rulesync", + relativeFilePath: "permissions.json", + fileContent: JSON.stringify({ + permission: { + read: { + ":project_roots": "deny", + "src/**": "allow", + }, + }, + }), + }); + + await CodexcliPermissions.fromRulesyncPermissions({ + outputRoot: testDir, + rulesyncPermissions, + logger, + }); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('":project_roots" is set as a direct filesystem access rule'), + ); + }); + + it("should skip empty string patterns with a warning", async () => { + const logger = createMockLogger(); + const rulesyncPermissions = new RulesyncPermissions({ + outputRoot: testDir, + relativeDirPath: ".rulesync", + relativeFilePath: "permissions.json", + fileContent: JSON.stringify({ + permission: { + read: { + "": "allow", + "src/**": "allow", + }, + }, + }), + }); + + const codexPermissions = await CodexcliPermissions.fromRulesyncPermissions({ + outputRoot: testDir, + rulesyncPermissions, + logger, + }); + + expect(logger.warn).toHaveBeenCalledWith("Skipping empty pattern in filesystem permissions."); + + const fileContent = codexPermissions.getFileContent(); + expect(fileContent).not.toContain('""'); + }); + it("should load existing .codex/config.toml", async () => { const codexDir = join(testDir, ".codex"); await ensureDir(codexDir); diff --git a/src/features/permissions/codexcli-permissions.ts b/src/features/permissions/codexcli-permissions.ts index 51d022f87..0144179f1 100644 --- a/src/features/permissions/codexcli-permissions.ts +++ b/src/features/permissions/codexcli-permissions.ts @@ -220,7 +220,7 @@ function convertRulesyncToCodexProfile({ if (Object.keys(projectRootFilesystem).length > 0) { if (typeof filesystem[CODEX_PROJECT_ROOTS_KEY] === "string") { logger?.warn( - `":project_roots" is set as a direct filesystem access rule in the permissions, but it will be overwritten by project-root rules. Consider removing the direct ":project_roots" entry.`, + `"${CODEX_PROJECT_ROOTS_KEY}" is set as a direct filesystem access rule in the permissions, but it will be overwritten by project-root rules. Consider removing the direct "${CODEX_PROJECT_ROOTS_KEY}" entry.`, ); } if (Object.keys(projectRootFilesystem).some((pattern) => pattern.includes("**"))) { @@ -290,7 +290,7 @@ function addFilesystemRule({ access: CodexFilesystemAccess; logger?: ToolPermissionsFromRulesyncPermissionsParams["logger"]; }): void { - if (pattern === "") { + if (pattern.trim() === "") { logger?.warn("Skipping empty pattern in filesystem permissions."); return; }