Skip to content

debug: OAuth probe (temporary, do not merge) - #1484

Closed
kyle-sexton wants to merge 5 commits into
mainfrom
debug/oauth-probe
Closed

debug: OAuth probe (temporary, do not merge)#1484
kyle-sexton wants to merge 5 commits into
mainfrom
debug/oauth-probe

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Temporary diagnostic for the review-lane incident. Will be closed and branch deleted.

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 33s —— View job


PR Review: debug/oauth-probe

Todos

  • Read changed files
  • Review for correctness, security, and CLAUDE.md alignment

Summary

This is a single-file temporary diagnostic workflow intended to probe OAuth token availability for claude-code-action. It is explicitly marked as not for merge by the PR author. The review below flags the one meaningful observation worth noting before closing.


Observations

.github/workflows/debug-oauth-probe.yml

Pinning looks correct
Both third-party actions are pinned to full commit SHAs with version comments — good hygiene.

  • actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
  • anthropics/claude-code-action@12531344451323133b0493233c759991ac61da12 # v1.0.174

id-token: write is broader than needed

L7

id-token: write is not used anywhere in the workflow — claude_code_oauth_token is supplied from a repository secret, not via OIDC token exchange. This permission is a no-op here but would be worth removing if this probe were ever promoted to a non-temporary file, since id-token: write allows any job step to request a JWT from GitHub's OIDC provider (a broader trust grant than needed).

Minor: outcome output uses expression syntax inside run:

L27

echo "outcome=${{ steps.cca.outcome }}"

steps.<id>.outcome is a GitHub Actions context expression — it resolves at template-expansion time before the shell runs, so the echo is fine. But for consistency with how EXECUTION_FILE is handled (env var → shell var), this could instead be:

env:
  CCA_OUTCOME: ${{ steps.cca.outcome }}
run: echo "outcome=$CCA_OUTCOME"

Neither is wrong; the current form just mixes two resolution styles in the same script block.


Verdict

No correctness or security blockers. The workflow does exactly what the description says — a scoped, low-risk diagnostic — and is appropriately marked for non-merge. The only thing worth noting for any future promoted form is dropping the unused id-token: write.

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 22s —— View job


Security Review — PR #1484

  • Gather context
  • Read changed files
  • Analyze for security vulnerabilities
  • Post findings

File reviewed: .github/workflows/debug-oauth-probe.yml


IMPORTANT — id-token: write granted without justification | Confidence: MEDIUM

Permalink: debug-oauth-probe.yml L7

id-token: write is granted at the workflow level, giving every step the ability to request an OIDC JWT signed by GitHub Actions. These tokens can be exchanged for credentials with any OIDC-aware cloud provider (AWS, GCP, Azure, etc.) or service that trusts GitHub's OIDC issuer.

No step in this workflow has any visible OIDC usage: actions/checkout doesn't require it, and the claude-code-action invocation passes a pre-existing OAuth token via secrets.CLAUDE_CODE_OAUTH_TOKEN rather than OIDC. If the action probes OIDC token issuance internally as part of its diagnostic purpose, that intent should be documented in a comment — otherwise this looks like an unintentional permission-widening change, which is one of the patterns this review is specifically hunting for.

For a pull_request-triggered workflow, fork PRs cannot obtain OIDC tokens regardless of this permission (GitHub blocks it), so the practical blast radius is limited to same-repo collaborators who can push branches. But the principle-of-least-privilege violation remains: if this workflow (or a copy of it) were later adapted to pull_request_target, this standing id-token: write grant would silently become exploitable.

Recommendation: Remove id-token: write unless it is explicitly required by the probe, and add a comment explaining why if it is.


SUGGESTION — Action execution file dumped to public log | Confidence: LOW

Permalink: debug-oauth-probe.yml L22–L32

The step at lines 22–32 reads the claude-code-action execution file and dumps fields including api_error_status, result (truncated to 600 chars), and timing/cost metadata to the workflow log. The prompt is controlled (Reply with exactly: ACTION_PROBE_OK), so the result text itself is low-risk. The concern is the api_error_status and any other diagnostic fields the action may populate on failure: if an OAuth or OIDC error response from the Anthropic API embeds a partial token, an endpoint URL with embedded credentials, or a WWW-Authenticate challenge containing sensitive material, it will appear in the public run log. This is speculative — API error bodies in this context are unlikely to contain secrets — but it is worth verifying that the execution file schema never surfaces token material before this probe pattern is reused.


What looks good

  • pull_request trigger (not pull_request_target): The correct, safe choice. Fork PRs cannot access repository secrets or OIDC tokens under this trigger. This eliminates the most common dangerous pattern for PR-triggered workflows.
  • Both actions are SHA-pinned: actions/checkout@9c091bb... and anthropics/claude-code-action@12531344... are pinned to immutable commit SHAs with version comments — good supply-chain hygiene.
  • EXECUTION_FILE used safely in shell: The value is assigned to an env var (env: EXECUTION_FILE: ${{ ... }}) and then expanded as ${EXECUTION_FILE:-}, not interpolated directly into the shell script as a GitHub expression. This sidesteps script injection via tainted output values.
  • steps.cca.outcome echo is safe: steps.<id>.outcome is a GitHub-controlled enum (success/failure/cancelled/skipped) — not user-controlled input — so the direct interpolation on line 27 carries no injection risk.
  • contents: read is minimal for the checkout step's needs.

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 26s —— View job


PR Review: debug/oauth-probe (sync — fd5442a)

Todos

  • Gather context
  • Review latest commit (fd5442a) diff
  • Post updated findings

This review covers the full PR diff. The latest commit (fd5442a — bisect: add track_progress) adds two parameters to the action invocation:

track_progress: true
claude_args: --model claude-sonnet-4-6 --max-turns 2

Observation: track_progress: true with no pull-requests: write

L21

track_progress: true tells claude-code-action to post progress comments back to the PR. That path uses the GitHub API to write a PR comment, which requires pull-requests: write on the GITHUB_TOKEN. The workflow's permissions block only declares contents: read and id-token: writepull-requests: write is absent.

Depending on the repo's default token permissions setting, this will either:

  • Fail silently — the action tries to post a comment, the API returns 403, the step continues (because continue-on-error: true), and the execution file records no comment-post activity.
  • Succeed anyway — if the org/repo default grants write to all workflow jobs regardless of the explicit permissions: block (not the GitHub default, but some orgs configure this).

If the bisect intent is "does track_progress: true change how the OAuth probe behaves end-to-end?", adding pull-requests: write to the declared permissions would make the experiment cleaner — it removes the variable of the comment-post failing silently from the observed outcome.

Fix this →


Carryover: id-token: write still unjustified

L7

Flagged in prior reviews and unchanged. No step exchanges an OIDC token — the action receives a pre-existing OAuth token from secrets.CLAUDE_CODE_OAUTH_TOKEN. This permission is a no-op here but is a standing least-privilege violation. Safe to drop.


What looks good

  • claude_args: --max-turns 2 — good diagnostic hygiene; bounds the cost and runtime of each probe run.
  • --model claude-sonnet-4-6 — explicit model pin avoids default-model drift affecting results across probe iterations.
  • continue-on-error: true — probe survives a failing action step; the Dump SDK result detail step (if: always()) still runs and captures the execution file.
  • SHA pinning on both actions — unchanged, still correct.
  • pull_request trigger (not pull_request_target) — unchanged, still the safe choice.
  • Path filter ['.github/workflows/debug-oauth-probe.yml'] — keeps the probe self-contained; only fires when this file changes.

Verdict

No correctness or security blockers for a temporary diagnostic. The main actionable note for the current bisect iteration: add pull-requests: write so the track_progress code path can actually complete its write, making the probe outcome unambiguous. The id-token: write should be dropped before any promoted form of this workflow.

@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 37s —— View job


Security Review — PR #1484

Todos

  • Read changed file
  • Analyze for security vulnerabilities
  • Post findings

File reviewed: .github/workflows/debug-oauth-probe.yml


IMPORTANT — id-token: write granted with no OIDC usage | Confidence: MEDIUM

Permalink: L7

id-token: write is set at the workflow level, granting every step the ability to request a signed OIDC JWT from GitHub's token endpoint. No step in this workflow performs OIDC token exchange: actions/checkout does not need it, and claude-code-action is supplied a pre-existing OAuth token via secrets.CLAUDE_CODE_OAUTH_TOKEN, not via OIDC.

The practical blast radius here is limited: because this workflow is triggered by pull_request (not pull_request_target), fork PRs cannot obtain OIDC tokens regardless of this grant. However, any same-repo collaborator who can push a branch and trigger this workflow can request a GitHub-signed OIDC JWT — which can be exchanged for cloud credentials at any OIDC-aware provider (AWS, GCP, Azure) that trusts GitHub's issuer. The stale permission is also a copy-paste trap: if this file were adapted to pull_request_target (a common escalation path when a team finds pull_request won't access secrets from forks), id-token: write would become silently exploitable.

Recommendation: Remove id-token: write; add it back with a comment if OIDC probing is actually the diagnostic goal.

Fix this →


SUGGESTION — EXECUTION_FILE path not confined to the runner workspace | Confidence: LOW

Permalink: L26–L33

steps.cca.outputs.execution_file is consumed as an env var and its value is passed directly to jq (and in the || fallback branch, to tail). There is no check that the path stays within $GITHUB_WORKSPACE or $RUNNER_TEMP before the file is opened for reading and its contents are printed to the log.

If claude-code-action were to emit a crafted execution_file output pointing outside the workspace (e.g., /proc/self/environ, /home/runner/.git-credentials, or a runner temp file containing a secret written by an earlier step), the jq parse would fail but the || tail -3 fallback would still print the last three lines of that file to the public run log.

Mitigated in practice by the SHA pin on anthropics/claude-code-action — the pinned version is trusted. But the pattern should be tightened for any reuse of this step:

case "${EXECUTION_FILE:-}" in
  "$GITHUB_WORKSPACE"/*|"$RUNNER_TEMP"/*) ;;
  *) echo "(execution file outside expected paths, skipping)"; exit 0 ;;
esac

SUGGESTION — api_error_status written to public log without redaction | Confidence: LOW

Permalink: L30

The jq filter prints api_error_status in full while capping result to 600 chars. On an auth failure, an Anthropic API error response body may include WWW-Authenticate challenge headers, diagnostic correlation IDs, or — in edge cases — partial token material in the error message. These would appear verbatim in the public run log. This is speculative and unlikely for normal Anthropic API error shapes, but the asymmetry (result is truncated; error status is not) is worth noting before this pattern is reused in a non-temporary context.


What looks good

  • pull_request trigger (not pull_request_target): Correct and safe. Fork PRs cannot access repository secrets under this trigger.
  • Both actions are SHA-pinned: actions/checkout@9c091bb... and anthropics/claude-code-action@12531344... are pinned to immutable commit SHAs with version comments — eliminates mutable-tag substitution risk.
  • EXECUTION_FILE passed via env:, not inline interpolation: The path is not interpolated directly into the run: script body; it is set as an env var and expanded via ${EXECUTION_FILE:-}. This correctly sidesteps shell-injection via tainted action output values.
  • steps.cca.outcome interpolation is safe: outcome is a runner-controlled enum (success/failure/cancelled/skipped) — not user-supplied — so the direct inline expression on line 28 carries no injection risk.
  • contents: read is appropriately minimal for the checkout step's needs.

@kyle-sexton
kyle-sexton deleted the debug/oauth-probe branch July 26, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant