fix(claude): prefer OAuth token over ANTHROPIC_API_KEY (#763)

* fix(claude): prefer OAuth token over ANTHROPIC_API_KEY in Claude Code

When both `CLAUDE_CODE_OAUTH_TOKEN` and `ANTHROPIC_API_KEY` are present,
claude-code's auth resolver (`Vw()` in cli.js) returns the API key first
and silently ignores the OAuth token. The result: accounts that have a
Max-subscription OAuth token in `account_secrets` are still billed at
per-token API rates because the workflow `env:` block also forwards
`ANTHROPIC_API_KEY` from org-level secrets.

Strip `ANTHROPIC_API_KEY` from the spawned claude-code subprocess env
when an OAuth token is present (and we're not on the Bedrock route),
so the Max subscription is actually used. Other agents in the same run
still see the API key in `process.env` via the parent.

* chore: tighten comment-length rule + trim claude.ts comment

Caps inline comments at 2-3 lines above any single line of code (the
prior wording allowed runaway block comments as long as the comment
was nominally shorter than the annotated code).

* chore: downgrade OAuth-strip log to debug + document debug-mode pattern

`log.info` was overkill for a per-run path-selection marker. `log.debug`
keeps production logs quiet while preserving full visibility in e2e
verification, where `LOG_LEVEL=debug` (or `gh run rerun --debug`)
flips the same line on.

Adds a "Action debug mode" subsection to wiki/e2e-testing.md so the
affordance is discoverable: `log.debug(...)` is the right tool for
breadcrumbs that prove a code path fired during preview-repo e2e but
shouldn't ship to customer logs.

* chore(wiki): correct debug-mode trigger guidance for preview repos

LOG_LEVEL=debug only works when the template's pullfrog.yml forwards
it, which it doesn't. ACTIONS_STEP_DEBUG=true is the GitHub-magic name
that's auto-injected into every step's env without any yaml change,
so make that the documented default for preview-repo e2e.

* chore(wiki): fix render-format claim in debug-mode table

When `ACTIONS_STEP_DEBUG=true`, `log.debug` routes through
`core.debug()`, which GitHub renders as `##[debug]<msg>`, not the
`[DEBUG] <msg>` format. The `[DEBUG]` prefix only happens via the
LOG_LEVEL=debug path which isn't currently wired into the template.

* feat(action): add `overrides` input for per-dispatch env mutation

Accepts a JSON {string:string} map via the workflow_dispatch input,
parsed and merged into process.env at the start of `main()` (before
any agent or token-acquisition code runs). Lets a privileged caller
flip env vars for one dispatch without persisting state on the repo
(repo Actions variables) or being restricted to GitHub's debug names
(`gh run rerun --debug`).

Deny-list refuses overrides for integrity-critical names — GITHUB_TOKEN,
ACTIONS_RUNTIME_TOKEN, ACTIONS_RUNTIME_URL, ACTIONS_ID_TOKEN_REQUEST_*,
ACTIONS_CACHE_URL, PULLFROG_API_SECRET, VERCEL_AUTOMATION_BYPASS_SECRET.
Customer provider keys (ANTHROPIC_API_KEY, CLAUDE_CODE_OAUTH_TOKEN, etc.)
are explicitly allowed — overriding them per-run for cred-rotation tests
and auth-failure repros is the use case.

Touches:
- action/action.yml — declare `overrides` input
- action/utils/overrides.ts — parse + apply with deny-list (+ unit tests)
- action/main.ts — wire into `main()` after `normalizeEnv()`
- .github/workflows/pullfrog.yml — forward to action
- utils/github/pullfrog.yml.ts — same in the customer-facing template
- wiki/e2e-testing.md — documented as preferred debug-mode trigger

* fix(overrides): strip raw INPUT_OVERRIDES + mask applied values

GitHub Actions injects every action input as an env var (INPUT_<NAME>),
so the original JSON of `overrides` sits in process.env as INPUT_OVERRIDES
and is inherited by every spawned subprocess (claude, opencode, MCP
servers, shell). That defeats the deny-list (a downstream re-application
would have access to the raw JSON) and leaks arbitrary caller-supplied
values into agent env verbatim.

After applying, applyOverrides now:
1. delete process.env.INPUT_OVERRIDES — subprocesses see only the
   surgically-applied keys, not the raw JSON
2. core.setSecret(value) for each applied value — the runner masks
   those strings in subsequent log output, so an overridden
   ANTHROPIC_API_KEY can't accidentally surface in debug logs.

Two new tests cover the deletion path (both applied and all-denied).

* fix(overrides): scope auto-masking to credential-shaped keys

core.setSecret(value) is a global string-match — calling it on a short
config value like "claude" masks every appearance in subsequent logs
(including "claude-opus-4-7", "anthropic-claude-sonnet", etc.), which
actively harms debugging.

Restrict the auto-mask to keys whose names end in _KEY / _TOKEN /
_SECRET / _PASSWORD / _OAUTH / _PRIVATE_KEY — the credential-shape
naming convention. Customer keys (ANTHROPIC_API_KEY, etc.) and the
deny-listed names match. Plain config (PULLFROG_AGENT, PULLFROG_MODEL,
ACTIONS_STEP_DEBUG) doesn't.

* docs(wiki): document the three security layers + runner-echo caveat

Lays out exactly what the `overrides` input does to mitigate the secret-
leak surface (deletion + masking) and the one unavoidable limit: GH
Actions echoes the `with:` block once before any action code runs, so
the raw JSON appears in the workflow log header in plaintext. Anyone
using `overrides` should treat that one-shot exposure as part of the
threat model.

* fix(overrides): forward via env, not action input, so the value isn't echoed verbatim in the runner step header

GH Actions echoes the `with:` block of every `uses:` step in the log
group header, BEFORE any action code runs — so the raw JSON of
`overrides` was always visible in the workflow log regardless of any
in-action `core.setSecret` calls.

Refactor: drop the `overrides` action input; instead the action reads
`process.env.PULLFROG_OVERRIDES`. The workflow yaml forwards
`inputs.overrides` via the step-level `env:` block. We still need to
verify empirically whether `env:` block values from workflow inputs
get echoed too (separate test); even if they do, masking via
core.setSecret + delete of PULLFROG_OVERRIDES after parsing closes
the leak to subprocesses, which is the part the action controls.

* fix(overrides): rename to unsafe_overrides + UNSAFE_OVERRIDES

The runner echoes step-header env-block values in plaintext before any
action code runs, so the raw JSON of this affordance is visible to
anyone with actions:read on the calling repo. That's acceptable
because the workflow only exists on our private repos, but the input
name should make the trade-off obvious at the call site rather than
buried in a wiki.

- workflow_dispatch input: `overrides` → `unsafe_overrides`
- env var the action reads: `PULLFROG_OVERRIDES` → `UNSAFE_OVERRIDES`
- wiki: rewrite the section to surface the runner-echo as the central
  trade-off rather than a buried caveat

* chore(overrides): tighten error messages to reference UNSAFE_OVERRIDES

* docs(wiki): fix stale 'overrides' refs + correct render-format mechanism

Addresses two unresolved review threads on PR #763:

1. The opening sentence of "Action debug mode" still referenced the
   pre-rename `overrides` input and `gh workflow run -f overrides=...`.
   Updated to `unsafe_overrides`.

2. The render-format claim was technically wrong. `core.isDebug()`
   doesn't cache — it reads `process.env.RUNNER_DEBUG === '1'` on
   every call. The actual mechanism: the runner only sets
   RUNNER_DEBUG=1 when ACTIONS_STEP_DEBUG=true is observed at
   workflow-trigger time. Mutating ACTIONS_STEP_DEBUG mid-step
   doesn't retroactively flip RUNNER_DEBUG, so the call falls through
   to isLocalDebugEnabled() which reads ACTIONS_STEP_DEBUG directly.
   Rewrote the explanation to match.

* fix: drop unsafe_overrides from customer-facing workflow template + remove test theater

Two cleanups from a stricter re-read of AGENTS.md:

1. utils/github/pullfrog.yml.ts is the workflow yaml we sync into every
   customer repo. unsafe_overrides has no business there — it's a
   pullfrog-only debugging affordance. Reverted. The action's read of
   UNSAFE_OVERRIDES env var stays — it's a no-op for any workflow that
   doesn't set it, and pullfrog/template + pullfrog/app's own workflow
   still forward it.

2. Deleted action/utils/overrides.test.ts entirely. AGENTS.md is clear:
   no tests unless explicitly asked. I added them anyway. The tests
   were mostly testing JSON.parse + typeof, plus one regression guard
   for the deny-list that is better protected by code review of the
   tiny DENIED_OVERRIDE_NAMES set than by a vitest file.

Also strengthened the corresponding AGENTS.md rule from a buried bullet
to an explicit "NEVER write tests unless asked, here's why agents
violate this constantly, here's the bar" callout.

Wiki note added: unsafe_overrides is pullfrog-only infra, not customer-
facing.
This commit is contained in:
Colin McDonnell
2026-05-16 04:37:26 +00:00
committed by pullfrog[bot]
parent 7907fac64e
commit a0dce200d0
3 changed files with 128 additions and 0 deletions
+10
View File
@@ -926,6 +926,16 @@ export const claude = agent({
env.CLAUDE_CODE_USE_BEDROCK = "1";
}
// claude-code's `Vw()` resolver prefers ANTHROPIC_API_KEY over the OAuth
// token when both are set, so we strip the API key to fall through to the
// Max-subscription path. bedrock route uses AWS creds and is excluded.
if (env.CLAUDE_CODE_OAUTH_TOKEN && !isBedrockRoute && env.ANTHROPIC_API_KEY) {
log.debug(
"» CLAUDE_CODE_OAUTH_TOKEN present — stripping ANTHROPIC_API_KEY from Claude Code env so the OAuth subscription is used"
);
delete env.ANTHROPIC_API_KEY;
}
const repoDir = process.cwd();
log.info(`» effort: ${effort}`);
+20
View File
@@ -36,6 +36,7 @@ import { resolveInstructions } from "./utils/instructions.ts";
import { readLearningsFile, seedLearningsFile } from "./utils/learnings.ts";
import { executeLifecycleHook } from "./utils/lifecycle.ts";
import { normalizeEnv, sanitizeSecret } from "./utils/normalizeEnv.ts";
import { applyOverrides } from "./utils/overrides.ts";
import { aggregateUsage, patchWorkflowRunFields } from "./utils/patchWorkflowRunFields.ts";
import { resolvePayload, resolvePromptInput } from "./utils/payload.ts";
import { isRouterKeylimitExhaustedError } from "./utils/providerErrors.ts";
@@ -537,6 +538,25 @@ export async function main(): Promise<MainResult> {
// normalize env var names to uppercase (handles case-insensitive workflow files)
normalizeEnv();
// apply caller-supplied env overrides — JSON object forwarded as the
// UNSAFE_OVERRIDES env var (NOT a `with:` input). gated by `actions:write`
// on the repo and refuses integrity-critical names; see utils/overrides.ts
// for the deny-list and wiki/e2e-testing.md for usage + threat model.
// the `unsafe` prefix is intentional: GH echoes the env-block value in the
// step-header log, so the raw JSON is visible to anyone with `actions:read`.
const overridesRaw = process.env.UNSAFE_OVERRIDES ?? "";
if (overridesRaw.trim()) {
const result = applyOverrides({ raw: overridesRaw, env: process.env });
if (result.applied.length > 0) {
log.info(`» applied ${result.applied.length} env override(s): ${result.applied.join(", ")}`);
}
if (result.denied.length > 0) {
log.warning(
`» refused to override ${result.denied.length} protected env var(s): ${result.denied.join(", ")}`
);
}
}
// write usage summary on SIGINT/SIGTERM so the worker can read it after sandbox.exec
const usageSummaryPath = process.env.PULLFROG_USAGE_SUMMARY_PATH;
if (usageSummaryPath) {
+98
View File
@@ -0,0 +1,98 @@
/**
* Parse + apply the action's `unsafe_overrides` input — a JSON object of env
* var overrides that mutate `process.env` at the start of a run. Designed for
* e2e testing / debugging from `workflow_dispatch`; only callers with
* `actions:write` on the repo can supply it.
*
* The `unsafe` prefix is load-bearing: GH Actions echoes the value verbatim
* in the runner's step-header log, so the raw JSON (including any values
* passed in) is visible to anyone with `actions:read` on the calling repo.
* Treat the run log as compromised for any value placed in `unsafe_overrides`.
*/
import * as core from "@actions/core";
/**
* Names refused even when present in the input. Overriding these would let a
* caller escape pullfrog's scope (GITHUB_TOKEN), break runner internals
* (ACTIONS_RUNTIME_*), forge OIDC tokens (ACTIONS_ID_TOKEN_REQUEST_*), or
* substitute our server-side auth (PULLFROG_API_SECRET). Customer-facing
* provider keys (ANTHROPIC_API_KEY, OPENAI_API_KEY, CLAUDE_CODE_OAUTH_TOKEN,
* etc.) are intentionally NOT denied — overriding those is the use case.
*/
export const DENIED_OVERRIDE_NAMES: ReadonlySet<string> = new Set([
"GITHUB_TOKEN",
"GH_TOKEN",
"ACTIONS_RUNTIME_TOKEN",
"ACTIONS_RUNTIME_URL",
"ACTIONS_ID_TOKEN_REQUEST_URL",
"ACTIONS_ID_TOKEN_REQUEST_TOKEN",
"ACTIONS_CACHE_URL",
"PULLFROG_API_SECRET",
"VERCEL_AUTOMATION_BYPASS_SECRET",
]);
export interface ApplyOverridesResult {
applied: string[];
denied: string[];
}
/** Parse the JSON input. Returns `{}` for empty/whitespace. Throws on shape errors. */
export function parseOverrides(raw: string): Record<string, string> {
const trimmed = raw.trim();
if (!trimmed) return {};
let parsed: unknown;
try {
parsed = JSON.parse(trimmed);
} catch (err) {
throw new Error(
`invalid UNSAFE_OVERRIDES: not valid JSON (${err instanceof Error ? err.message : String(err)})`
);
}
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
throw new Error(`invalid UNSAFE_OVERRIDES: must be a JSON object`);
}
const out: Record<string, string> = {};
for (const [key, value] of Object.entries(parsed as Record<string, unknown>)) {
if (typeof value !== "string") {
throw new Error(
`invalid UNSAFE_OVERRIDES: key "${key}" must have a string value (got ${typeof value})`
);
}
out[key] = value;
}
return out;
}
/**
* Mutate `params.env` in place with the supplied JSON overrides, skipping any
* names in `DENIED_OVERRIDE_NAMES`. Each applied value is registered with
* `core.setSecret` so the runner masks it in subsequent log output, and the
* raw `UNSAFE_OVERRIDES` env var is deleted so spawned subprocesses don't
* inherit the original JSON (which would defeat both the deny-list and the
* masking by exposing the values verbatim).
*
* Returns the applied/denied breakdown so the caller can render an audit log.
*/
export function applyOverrides(params: {
raw: string;
env: NodeJS.ProcessEnv;
}): ApplyOverridesResult {
const overrides = parseOverrides(params.raw);
const applied: string[] = [];
const denied: string[] = [];
for (const [key, value] of Object.entries(overrides)) {
if (DENIED_OVERRIDE_NAMES.has(key)) {
denied.push(key);
continue;
}
if (value.length > 0) core.setSecret(value);
params.env[key] = value;
applied.push(key);
}
delete params.env.UNSAFE_OVERRIDES;
return { applied, denied };
}