Skip to content

fix: pin bun runtime config and improve log hygiene #1174

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

Merged
ashwin-ant merged 2 commits into from
Apr 5, 2026
Merged

fix: pin bun runtime config and improve log hygiene #1174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3976f9c
fix: pin bun runtime config and improve log hygiene
ashwin-ant Apr 5, 2026
17a8287
snapshot all SENSITIVE_PATHS to .claude-pr/, not just .claude/
ashwin-ant 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
15 changes: 12 additions & 3 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,10 @@ runs:
id: run
shell: bash
run: |
bun run ${GITHUB_ACTION_PATH}/src/entrypoints/run.ts
bun --no-env-file \
--config="${GITHUB_ACTION_PATH}/bunfig.toml" \
--tsconfig-override="${GITHUB_ACTION_PATH}/tsconfig.json" \
Comment on lines +230 to +232
Copy link
Copy Markdown
Contributor

@claude claude Bot Apr 5, 2026

Choose a reason for hiding this comment

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

🟣 The positional entrypoint path argument is unquoted in all three bun run steps: run ${GITHUB_ACTION_PATH}/src/entrypoints/run.ts — if GITHUB_ACTION_PATH contains spaces (possible on self-hosted runners), bash word-splitting will break the command. This is a pre-existing issue that this PR makes more visible by adding properly double-quoted --config and --tsconfig-override flags alongside the unquoted path; fix by wrapping the path: run "${GITHUB_ACTION_PATH}/src/entrypoints/run.ts".

Extended reasoning...

The unquoted shell expansion run ${GITHUB_ACTION_PATH}/src/entrypoints/run.ts is present in all three bun entrypoint steps (lines ~232, ~332, ~346 of action.yml). When bash processes this command, it performs word-splitting on unquoted variable expansions. If GITHUB_ACTION_PATH contains a space — for example /my runner/_actions/org/repo/v1 on a self-hosted runner — bash will split it into separate tokens, and bun will receive run, /my, runner/... as distinct arguments rather than a single file path, causing a file-not-found error.

The code path that triggers the bug: any self-hosted GitHub Actions runner whose configured path contains whitespace. The environment variable GITHUB_ACTION_PATH is set by the Actions runner to the directory of the checked-out action. While GitHub-hosted runners use /home/runner/work/_actions/... (no spaces), enterprise self-hosted runners can be configured with arbitrary paths.

The existing code fails to prevent this because the --config and --tsconfig-override flags added by this PR are correctly quoted (--config="${GITHUB_ACTION_PATH}/bunfig.toml"), demonstrating that the author was aware of proper quoting, but the final positional argument was overlooked. The old single-line invocation had the same issue and was never fixed.

The practical impact is low: GitHub-hosted runners never have spaces in GITHUB_ACTION_PATH, and most self-hosted runners follow similar conventions. However, the inconsistency is now more prominent — three properly-quoted flags sit right above an unquoted path argument, which could mislead reviewers into thinking the path is safe.

This is a pre-existing issue not introduced by this PR. The original bun run ${GITHUB_ACTION_PATH}/src/entrypoints/run.ts had the same unquoted expansion. The PR touched these lines and could have fixed it incidentally when reformatting to multi-line, but did not introduce the problem.

Step-by-step proof: (1) A self-hosted runner is configured with a path containing a space, e.g. /my runners/actions. (2) The runner sets GITHUB_ACTION_PATH=/my runners/actions/org/repo@v1. (3) bash executes bun --no-env-file --config="..." --tsconfig-override="..." run ${GITHUB_ACTION_PATH}/src/entrypoints/run.ts. (4) After word-splitting, bun receives: arg0=run, arg1=/my, arg2=runners/actions/org/repo@v1/src/entrypoints/run.ts. (5) Bun tries to find a file named /my — it does not exist — and exits with an error. Fix: change to run "${GITHUB_ACTION_PATH}/src/entrypoints/run.ts" in all three steps.

run ${GITHUB_ACTION_PATH}/src/entrypoints/run.ts
env:
# Prepare inputs
MODE: ${{ inputs.mode }}
Expand Down Expand Up @@ -324,7 +327,10 @@ runs:
if: always() && inputs.ssh_signing_key != ''
shell: bash
run: |
bun run ${GITHUB_ACTION_PATH}/src/entrypoints/cleanup-ssh-signing.ts
bun --no-env-file \
--config="${GITHUB_ACTION_PATH}/bunfig.toml" \
--tsconfig-override="${GITHUB_ACTION_PATH}/tsconfig.json" \
run ${GITHUB_ACTION_PATH}/src/entrypoints/cleanup-ssh-signing.ts

- name: Post buffered inline comments
if: always() && inputs.classify_inline_comments != 'false'
Expand All @@ -336,7 +342,10 @@ runs:
PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }}
ANTHROPIC_API_KEY: ${{ inputs.anthropic_api_key }}
run: |
bun run ${GITHUB_ACTION_PATH}/src/entrypoints/post-buffered-inline-comments.ts
bun --no-env-file \
--config="${GITHUB_ACTION_PATH}/bunfig.toml" \
--tsconfig-override="${GITHUB_ACTION_PATH}/tsconfig.json" \
run ${GITHUB_ACTION_PATH}/src/entrypoints/post-buffered-inline-comments.ts

- name: Revoke app token
if: always() && inputs.github_token == '' && steps.run.outputs.github_token != '' && steps.run.outputs.skipped_due_to_workflow_validation_mismatch != 'true'
Expand Down
2 changes: 1 addition & 1 deletion base-action/src/run-claude-sdk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export async function runClaudeWithSdk(

console.log(`Running Claude with prompt from file: ${promptPath}`);
// Log SDK options without env (which could contain sensitive data)
const { env, ...optionsToLog } = sdkOptions;
const { env, extraArgs, ...optionsToLog } = sdkOptions;
console.log("SDK options:", JSON.stringify(optionsToLog, null, 2));

const messages: SDKMessage[] = [];
Expand Down
4 changes: 1 addition & 3 deletions base-action/test/parse-sdk-options.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,9 +343,7 @@ describe("parseSdkOptions", () => {
const result = parseSdkOptions(options);

expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-haiku");
expect(result.sdkOptions.extraArgs?.["prompt"]).toBe(
"use color #ff0000",
);
expect(result.sdkOptions.extraArgs?.["prompt"]).toBe("use color #ff0000");
});
});

Expand Down
2 changes: 2 additions & 0 deletions bunfig.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Intentionally minimal. action.yml pins --config to this file so bun resolves
# its runtime config from the action directory rather than the workspace.
20 changes: 13 additions & 7 deletions src/github/operations/restore-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ const SENSITIVE_PATHS = [
".claude.json",
".gitmodules",
".ripgreprc",
"CLAUDE.md",
"CLAUDE.local.md",
".husky",
Copy link
Copy Markdown

@maxired maxired Apr 9, 2026

Choose a reason for hiding this comment

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

In our repo, we do commit the .husky files, but not the hook setup in themself.
with this setup, we need to rerun npx husky to reinstall the hooks, otherwise, our pre commit hook do not run.
this broke previous behavior

];

/**
Comment thread
claude[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -44,16 +47,19 @@ export function restoreConfigFromBase(baseBranch: string): void {
`Restoring ${SENSITIVE_PATHS.join(", ")} from origin/${baseBranch} (PR head is untrusted)`,
);

// Snapshot the PR's .claude/ tree to .claude-pr/ before deleting it.
// This lets review agents inspect what the PR actually changes (CLAUDE.md,
// settings, hooks, MCP configs) without those files ever being executed.
// The snapshot is taken before the security delete so it captures the
// Snapshot every PR-authored sensitive path into .claude-pr/ before deletion
// so review agents can inspect what the PR changes without those files ever
// being executed. Captured before the security delete so it reflects the
// PR-authored version.
rmSync(".claude-pr", { recursive: true, force: true });
if (existsSync(".claude")) {
cpSync(".claude", ".claude-pr", { recursive: true });
for (const p of SENSITIVE_PATHS) {
if (existsSync(p)) {
cpSync(p, `.claude-pr/${p}`, { recursive: true });
}
}
if (existsSync(".claude-pr")) {
console.log(
"Preserved PR's .claude/ → .claude-pr/ for review agents (not executed)",
"Preserved PR's sensitive paths → .claude-pr/ for review agents (not executed)",
);
}

Expand Down
1 change: 1 addition & 0 deletions src/github/token.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ export async function setupGitHubToken(): Promise<string> {
},
);
console.log("App token successfully obtained");
core.setSecret(appToken);

console.log("Using GITHUB_TOKEN from OIDC");
return appToken;
Comment on lines 148 to 154
Copy link
Copy Markdown
Contributor

@claude claude Bot Apr 5, 2026

Choose a reason for hiding this comment

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

🔴 The OIDC token obtained via getOidcToken() is never masked with core.setSecret(), while this PR adds core.setSecret(appToken) for the exchanged app token. Although the OIDC token is not currently logged in any code path, the PR establishes an inconsistent masking pattern — add core.setSecret(oidcToken) immediately after line 138 to apply the same hygiene to the upstream bearer credential.

Extended reasoning...

What the bug is: The setupGitHubToken() function obtains an OIDC JWT token from GitHub Actions at line 138 (const oidcToken = await retryWithBackoff(() => getOidcToken())) and then uses it to exchange for an app token. This PR adds core.setSecret(appToken) at line 151 to mask the exchanged app token in CI logs. However, the oidcToken itself — a GitHub Actions JWT bearer credential with audience claude-code-github-action — is never passed to core.setSecret(), meaning it is not registered as a secret and would appear in plaintext if it ever surfaced in logs.

The specific code path: The OIDC token flows from getOidcToken() into retryWithBackoff(), then into exchangeForAppToken() where it is placed in the Authorization: Bearer ${oidcToken} header. The PR masks the downstream credential (appToken) but not the upstream credential (oidcToken).

Why existing code does not prevent it: The current code does not log the OIDC token directly — retryWithBackoff logs only lastError.message (not full error objects or arguments), and error paths in exchangeForAppToken throw Error objects with plain message strings derived from the server response, not from the token value. So the token is not actively leaking today.

What the impact would be: While not an active leak in current code, the OIDC token is still a valid bearer credential (short-lived but non-trivially scoped to the action's audience). More importantly, the PR explicitly establishes the pattern of calling core.setSecret() for sensitive tokens — applying it to the downstream token but not the upstream one is an inconsistency that could cause confusion. If any future logging, debug output, or error-handling change inadvertently surfaces oidcToken, it would appear in plaintext in publicly-visible GitHub Actions logs. GitHub's own Actions toolkit documentation recommends masking tokens returned by core.getIDToken() immediately after retrieval.

How to fix it: Add core.setSecret(oidcToken) immediately after line 138, right after the console.log('OIDC token successfully obtained') statement. This completes the security hygiene pass the PR is already performing.

Step-by-step proof of the inconsistency: (1) setupGitHubToken() calls getOidcToken() and gets back a JWT string. (2) This JWT is stored in oidcToken at line 138 — no core.setSecret() call is made. (3) oidcToken is passed to exchangeForAppToken() where it becomes the Authorization header value. (4) exchangeForAppToken() returns appToken. (5) The PR adds core.setSecret(appToken) at line 151. (6) Result: appToken is masked in GitHub Actions logs; oidcToken is not. If any log statement were added to print oidcToken (e.g., a debug log in exchangeForAppToken), the full JWT would appear unredacted.

Expand Down
Loading