fix: handle 404 in checkHumanActor for bot/app actors#1
Closed
saarakelyan wants to merge 49 commits into
Closed
Conversation
…nthropics#1066) * Restore .claude/ and .mcp.json from PR base branch before CLI runs The CLI's non-interactive mode trusts cwd: it reads .mcp.json and .claude/settings{,.local}.json from the working directory and acts on them before any tool-permission gating — executing hooks, setting env vars (NODE_OPTIONS, LD_PRELOAD), running apiKeyHelper shell commands, and auto-approving MCP servers. When this action checks out a PR head, these files are attacker-controlled. Rather than enumerate dangerous keys, replace the entire .claude/ tree and .mcp.json with the versions from the PR base branch (which a maintainer has reviewed). Paths absent on base are deleted. Uses local git state, so no TOCTOU against the GitHub API. * Read PR base ref from payload for config restore in agent mode Agent mode's branchInfo.baseBranch defaults to "main" (or env/input override) instead of the PR's actual target branch — it doesn't query prData.baseRefName like tag mode does. This meant a PR targeting develop would get .claude/ restored from main. Fix by reading pull_request.base.ref directly from the webhook payload for pull_request, pull_request_review, and pull_request_review_comment events. For issue_comment on a PR (no base.ref in payload), fall back to the mode-provided value — tag mode's value is correct (from GraphQL); agent mode on issue_comment is an edge case that at worst restores from the wrong trusted branch, which is still secure. The payload value passes through validateBranchName for defense-in-depth (GitHub enforces valid branch names server-side, but we validate anyway). * Extend restored paths to .gitmodules, .ripgreprc, .claude.json .gitmodules defines submodule URLs and paths; path-confusion attacks against git submodule operations can write into .git/hooks. .ripgreprc can set --pre (arbitrary command on each file) if RIPGREP_CONFIG_PATH points at it. .claude.json is cheap defense-in-depth. Documented why .git/ is excluded (not trackable in commits, and restoring it would undo the PR checkout), along with .gitconfig (git never reads it from cwd) and shell rc files (sourced from $HOME, not cwd — checkout cannot reach $HOME).
…red (anthropics#1093) * Auto-set CLAUDE_CODE_SUBPROCESS_ENV_SCRUB when allowed_non_write_users is configured Sets the env var automatically whenever allowed_non_write_users is non-empty, so downstream workflows don't need to add it manually. Updates the input description and docs/security.md to note the behavior. :house: Remote-Dev: homespace * Fall back to inherited env when allowed_non_write_users is unset :house: Remote-Dev: homespace * Let workflow/job env override the auto-set scrub flag Env var takes priority so users can opt in/out via CLAUDE_CODE_SUBPROCESS_ENV_SCRUB at job or workflow level independently of allowed_non_write_users. :house: Remote-Dev: homespace
…1132) - Add optional bubblewrap setup step for Linux subprocess isolation when allowed_non_write_users is configured - Use git credential helper instead of embedding token in remote URL - edit-issue-labels.sh: read issue number from workflow event payload instead of CLI arg - Add CLAUDE_CODE_SCRIPT_CAPS env for per-script call limit config - docs/security.md: note recommended github_token configuration :house: Remote-Dev: homespace
…thropics#1143) * fix: fall back to repo default_branch instead of hardcoded "main" When no explicit base_branch input is provided, the action previously fell back to a hardcoded "main", which fails on repositories whose default branch is named differently (e.g. "master", "develop"). This reads repository.default_branch from the GitHub event payload (populated once in parseGitHubContext) and uses it as the fallback in all three callsites: agent/index.ts, run.ts, and update-comment-link.ts. Explicit env/input precedence is preserved; "main" remains only as a last-resort defensive fallback if the payload somehow lacks the field. * test: drop unused BASE_BRANCH env handling from default_branch test agent/index.ts no longer reads process.env.BASE_BRANCH directly (it now goes through context.inputs.baseBranch which is set on the mock context), so saving/clearing/restoring that env var in the regression test is dead code.
env context isn't available in composite-action if: conditions. Move opt-out check into run: body. :house: Remote-Dev: homespace
Bun's execFileSync without an explicit env option spawns with the process startup environment, dropping runtime process.env mutations. The credential helper reads GH_TOKEN which is set at runtime, so git fetch in the restore-config path failed with empty password. Fixes anthropics#1139 🏠 Remote-Dev: homespace
Add a check for non-empty github_token output before attempting to revoke the app token in the cleanup step. When the prepare phase fails (e.g., unsupported event type with track_progress), no token is acquired, causing the cleanup curl to send an empty Bearer token and produce a confusing "Bad credentials" 401 error. Fixes anthropics#858 Co-authored-by: Dave-London <hello@os4us.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…thropics#1125) * Use env vars for workflow_run context values in example workflows * Add security note to ci-failure-auto-fix example about trust requirements
) * docs: document include_comments_by_actor and exclude_comments_by_actor inputs These inputs were added in anthropics#812 but never documented in usage.md or security.md. This adds them to the inputs table in usage.md and references comment filtering as a prompt injection mitigation in security.md. Fixes anthropics#972 * docs: clarify wildcard support is limited to *[bot] pattern Address review feedback: "Supports wildcards" was misleading since only the *[bot] pattern is supported, not general glob matching.
…1034) The reviewData variable is typed as `{ nodes: GitHubReview[] } | null`, but the fallback value was `[]` (a plain array). When `pullRequest.reviews` is null/undefined, `reviewData` becomes `[]`, causing `reviewData.nodes` to return `undefined` instead of `[]`. This leads to silent failures in downstream code that iterates over `reviewData.nodes`, such as `filterReviewsToTriggerTime` and `filterCommentsByActor`. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
When id-token: write permission is enabled, ACTIONS_ID_TOKEN_REQUEST_URL and ACTIONS_ID_TOKEN_REQUEST_TOKEN are passed to the Claude session via the process.env spread in parseSdkOptions(). This allows Claude to mint new OIDC tokens, which is an unintended capability. This commit deletes these two variables from the env object before passing it to the Claude SDK. The OIDC flow in token.ts reads directly from process.env and runs before parseSdkOptions(), so it is unaffected. Fixes anthropics#1010
…opics#1082) Add shouldRetry predicate to RetryOptions so callers can abort retries for errors that will never succeed (e.g. 401 WorkflowValidationSkipError). Previously, retryWithBackoff retried all errors blindly, wasting ~35s on deterministic failures like workflow validation 401s. Fixes anthropics#1081 Co-authored-by: Claude <noreply@anthropic.com>
…hropics#1163) bun install --production strips execute bits from vendored binaries (bun bug). The Claude Agent SDK ships rg binaries in: node_modules/@anthropic-ai/claude-agent-sdk/vendor/ripgrep/ {x64,arm64}-{linux,darwin}/rg {x64,arm64}-win32/rg.exe After bun --production, all of these lose +x, causing EACCES when the SDK tries to spawn ripgrep. The fix is a targeted find(1) that restores +x on the rg binaries immediately after bun install. Design notes: - -type f excludes symlinks (symlink attack safety, no || true needed) - -name "rg" naturally excludes rg.exe on Windows (find returns nothing, chmod never called — safe and correct on all platforms) - .node audio-capture files use dlopen, not exec — no +x needed there - Fails loudly if the binary path is missing (no || true) so a SDK packaging change is immediately visible rather than silently broken Fixes anthropics#1140 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…pics#1167) `validateBranchName` used a strict whitelist that excluded `#`, causing the action to fail on PRs from branches like `put-back-arm64-anthropics#2` with "Invalid branch name" — even though the branch already exists in git and `#` is permitted by git-check-ref-format. The validation was designed to prevent command injection. However, every git call in the action uses `execFileSync`, which bypasses the shell entirely and passes arguments directly to the kernel's execve. There is no shell to interpret `#` as a metacharacter, so the strict whitelist was over-blocking valid names with no security benefit. Add `#` to the whitelist pattern, and update the JSDoc and error message to reflect the allowed character set. Fixes anthropics#1137. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…nthropics#1166) When a PR head contains `.gitmodules`, git's default `fetch.recurseSubmodules=on-demand` config causes `git fetch` to attempt submodule object fetches. In CI (no credentials), this blocks indefinitely waiting for auth — producing ~4-hour hangs reported in anthropics#1088. Two changes, both defence-in-depth: 1. Delete SENSITIVE_PATHS *before* fetching. The attacker-controlled `.gitmodules` is absent during the network operation, so git never sees a submodule config to follow regardless of git settings. 2. Pass `--no-recurse-submodules` to the fetch. Suppresses submodule fetching explicitly, independent of any git config on the runner. The original order (fetch-then-delete) was a brief window where `.gitmodules` from the PR head could influence the fetch. Reordering also tightens the security property: if `git checkout` below fails, the attacker-controlled file is already gone rather than present during fetch. Fixes anthropics#1088. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…#1055) shell-quote treats # as a shell comment character, swallowing all subsequent content including flags on new lines. Strip comment lines (lines starting with #) before passing input to shell-quote. Fixes anthropics#802 Co-authored-by: VoidChecksum <Admin@CyberNord>
…nthropics#1172) When a PR modifies files under .claude/, the security restore in restoreConfigFromBase() overwrites them with the base branch version — correct for execution safety, but it means review agents never see what the PR actually changes. Before deleting the PR-controlled .claude/ tree, copy it to .claude-pr/. Review agents can read .claude-pr/ to inspect the PR's hooks, MCP configs, settings, and CLAUDE.md without those files ever being executed. The snapshot is taken before the security delete so it captures the full PR-authored version. Fixes anthropics#1134. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: pin bun runtime config and improve log hygiene * snapshot all SENSITIVE_PATHS to .claude-pr/, not just .claude/
…action step (anthropics#1162) These three env vars are read directly from process.env by the Claude CLI subprocess to configure MCP server behavior. Users setting them in their workflow had no reliable way to make them reach the CLI: - Job-level env: shadowed by the step's explicit env: block - Step-level env: on the calling workflow step is not inherited by composite action steps - GITHUB_ENV from a prior step: same shadowing problem - settings input: writes to ~/.claude/settings.json, not process.env The fix is to add explicit ${{ env.VAR }} passthrough lines for all three vars, matching the existing pattern already used for OTEL_*, AWS_*, and Vertex configuration (lines 271-317). No TypeScript changes are needed; the forwarding chain in parse-sdk-options.ts is already correct. Fixes anthropics#1152 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…rs (anthropics#1185) dirname() preserves embedded newlines, so a value like `/usr/bin/claude\n/attacker/path` writes two lines to GITHUB_PATH, injecting an attacker-controlled directory into PATH for all subsequent workflow steps. Validate the input immediately after reading it and throw if it contains any control characters (0x00-0x1f, 0x7f). This is fail-closed rather than silent stripping — a path with control characters is always misconfigured or malicious. Fixes anthropics#1160 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…set (anthropics#1190) Copies the bun binary into $GITHUB_ACTION_PATH/bin before the claude step runs and uses that copy in the two post-steps that invoke bun. Falls back to PATH-resolved bun when allowed_non_write_users is empty. :house: Remote-Dev: homespace
…nthropics#1208) Ensures later steps resolve standard tools like git and tar from /usr/bin regardless of what setup actions added earlier in the job. Also strengthens the PAT guidance in security.md. :house: Remote-Dev: homespace
Bot actors like github-merge-queue[bot] can't be looked up via
GET /users/{login} — the API returns 404. Previously this crashed
the action before it could check the allowed_bots list.
Now catches 404 and treats the actor as a Bot, allowing the
existing allowedBots logic to decide whether to proceed.
Made-with: Cursor
|
This PR adds or modifies If this is a new flow, please make sure you actually need See existing workflows in this repo for safe usage examples, or contact the AppSec team. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
checkHumanActorcallsoctokit.users.getByUsername()to look up the workflow actor, but bot/app actors likegithub-merge-queue[bot]can't be resolved via the Users API — it returns 404allowed_botslist"Bot"type, allowing the existingallowedBotslogic to decide whether to proceedContext
This breaks all
schedule-triggered workflows using agent mode. For scheduled runs, GitHub sets the actor to the last committer of the workflow file — often a bot likegithub-merge-queue[bot]. TheGET /users/github-merge-queuecall returns 404, and the error propagates uncaught.Manual triggers (
workflow_dispatch) work fine because the actor is a human user.Test plan
github-merge-queueactor on scheduled workflowsallowedBotsas beforeMade with Cursor