Skip to content

fix: merge project permissions.allow into tag mode --allowedTools #1173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
qozle wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: merge project permissions.allow into tag mode --allowedTools #1173

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbdaa1f
fix: merge project permissions.allow into tag mode --allowedTools
qozle Apr 5, 2026
fc5ea6e
fix: address review — scope and quoting issues in project permissions…
qozle Apr 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/entrypoints/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,34 @@ async function run() {
}
if (restoreBase) {
restoreConfigFromBase(restoreBase);

// After restore, .claude/settings.json is the trusted base-branch version.
// For tag mode, merge its permissions.allow entries into claudeArgs so that
// project-approved tools (e.g. Bash(pnpm test:*)) are not silently denied.
// PR #1002 hardened tag mode with an explicit --allowedTools allowlist, which
// inadvertently stopped respecting project settings entirely.
if (modeName === "tag") {
Copy link
Copy Markdown
Collaborator

@ashwin-ant ashwin-ant Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only runs when restoreBase is set (PR-triggered tag mode). Issue-triggered tag mode also has the #1002 hardening but no restore, so project permissions.allow stays ignored there. The settings file is equally trusted in that case (default branch checkout). Suggest moving this block to just after the if (restoreBase) { ... } closes, still gated on modeName === "tag".

try {
const settingsPath = ".claude/settings.json";
if (existsSync(settingsPath)) {
const settings = JSON.parse(readFileSync(settingsPath, "utf-8"));
const projectAllow: unknown = settings?.permissions?.allow;
if (Array.isArray(projectAllow) && projectAllow.length > 0) {
const projectTools = projectAllow.filter(
(t): t is string => typeof t === "string" && t.length > 0,
);
if (projectTools.length > 0) {
prepareResult.claudeArgs += ` --allowedTools "${projectTools.join(",")}"`;
Copy link
Copy Markdown
Collaborator

@ashwin-ant ashwin-ant Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String-concatenating into claudeArgs means this goes through shell-quote parsing in base-action. A tool entry containing " or , would break. Safer to thread the project tools through to prepareTagMode's tool builder so they're added to the array before it's serialized. If that's too invasive for this PR, at minimum escape " in the joined string.

console.log(
`Merged ${projectTools.length} project permission(s) from .claude/settings.json into tag mode tools`,
);
}
}
}
} catch {
// Malformed settings.json — proceed with hardcoded tool list only.
}
}
}
}

Expand Down