Skip to content

chore: Cherry-picked changes from upstream #42

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
Raj-StepSecurity merged 3 commits into from
Aug 3, 2026
Merged

chore: Cherry-picked changes from upstream #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e95930
chore: Cherry-picked changes from upstream
Raj-StepSecurity Jul 30, 2026
c718e09
rebased
Raj-StepSecurity Jul 30, 2026
f976ccf
conflicted commits cherry-picked
Raj-StepSecurity Jul 30, 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
2 changes: 1 addition & 1 deletion base-action/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ runs:
PATH_TO_CLAUDE_CODE_EXECUTABLE: ${{ inputs.path_to_claude_code_executable }}
run: |
if [ -z "$PATH_TO_CLAUDE_CODE_EXECUTABLE" ]; then
CLAUDE_CODE_VERSION="2.1.203"
CLAUDE_CODE_VERSION="2.1.220"
echo "Installing Claude Code v${CLAUDE_CODE_VERSION}..."
for attempt in 1 2 3; do
echo "Installation attempt $attempt..."
Expand Down
20 changes: 10 additions & 10 deletions base-action/bun.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion base-action/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
},
"dependencies": {
"@actions/core": "^2.0.3",
"@anthropic-ai/claude-agent-sdk": "0.3.203",
"@anthropic-ai/claude-agent-sdk": "^0.3.220",
"axios": "^1.16.1",
"shell-quote": "^1.8.4"
},
Expand Down
3 changes: 2 additions & 1 deletion base-action/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ async function run() {
core.setOutput("conclusion", "failure");
process.exit(1);
} finally {
// Stop refreshing the workload identity token file so the process can exit
// Stop refreshing the workload identity token file (so the process can
// exit) and delete the token material so it doesn't outlive this step

@claude claude Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This comment now documents an invariant the code doesn't hold: process.exit(1) at line 126 doesn't unwind the stack, so on every failure path of the standalone base-action this finally never runs and the token dir is left behind.

That was tolerable before (the leftover was a single-use OIDC JWT that fails with jti_reused), but this PR makes the same directory hold the SDK's cached exchanged Anthropic credential — a reusable bearer token valid until expires_at. RUNNER_TEMP is per-job, so any later step in the same job can read it.

core.setFailed already forces a non-zero exit, so dropping the explicit process.exit(1) is the smallest fix. (An always() cleanup step in action.yml — the convention this repo already uses for token revocation and SSH signing — would also cover SIGTERM/cancellation, which no finally catches in either entrypoint.)

workloadIdentity?.stop();
}
}
Expand Down
5 changes: 4 additions & 1 deletion base-action/src/parse-sdk-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ export function parseSdkOptions(options: ClaudeOptions): ParsedSdkOptions {
// Detect if --json-schema is present (for hasJsonSchema flag)
const hasJsonSchema = "json-schema" in extraArgs;

const modelFromClaudeArgs = extraArgs["model"] || undefined;
delete extraArgs["model"];
Comment on lines +204 to +205

@claude claude Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This inverts CLI precedence: an explicit --model in claude_args now silently loses to an ambient env var.

--model is deleted from extraArgs (so it never reaches the CLI) and is only a fallback at line 310: model: options.model || modelFromClaudeArgs. But options.model is process.env.ANTHROPIC_MODEL (base-action/src/index.ts:106, src/entrypoints/run.ts:338) — an ambient env var, not an explicit input. A CLI flag beating an env var is the normal expectation; this does the opposite, with nothing logged.

Scenario: a workflow sets env: ANTHROPIC_MODEL: <opus> at the job level and one step passes claude_args: --model <haiku> for a cheap task. Previously the flag was passed through to the CLI; now it's dropped and opus runs, at ~10× the cost, with no indication in the logs.

Suggest modelFromClaudeArgs || options.model, or at minimum a core.warning() when both are set.


const additionalDirectories = extraArgs["add-dir"]
? extraArgs["add-dir"]
.split(ACCUMULATE_DELIMITER)
Expand Down Expand Up @@ -304,7 +307,7 @@ export function parseSdkOptions(options: ClaudeOptions): ParsedSdkOptions {
// Build SDK options - use merged tools from both direct options and claudeArgs
const sdkOptions: SdkOptions = {
// Direct options from ClaudeOptions inputs
model: options.model,
model: options.model || modelFromClaudeArgs,
maxTurns: options.maxTurns ? parseInt(options.maxTurns, 10) : undefined,
allowedTools:
mergedAllowedTools.length > 0 ? mergedAllowedTools : undefined,
Expand Down
18 changes: 14 additions & 4 deletions base-action/src/run-claude-sdk.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,10 @@ export async function runClaudeWithSdk(
throw new Error("No result message received from Claude");
}

const isSuccess = resultMessage.subtype === "success";
// subtype "success" with is_error:true means the run errored without producing
// a real result — treat it as failure so CI does not show a misleading green check.
const isSuccess =
resultMessage.subtype === "success" && !resultMessage.is_error;
result.conclusion = isSuccess ? "success" : "failure";

// Handle structured output
Expand All @@ -234,14 +237,21 @@ export async function runClaudeWithSdk(
}

if (!isSuccess) {
if (resultMessage.subtype === "success" && resultMessage.is_error) {

@claude claude Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This new diagnostic is unreachable when --json-schema is in play, and the user gets a misleading error instead.

Before this change subtype === "success" implied isSuccess, so the hasJsonSchema && !isSuccess && subtype === "success" combination couldn't happen. It can now — and the hasJsonSchema block at lines 218-237 throws first with --json-schema was provided but Claude did not return structured_output. Result subtype: success, which reads as "the model just didn't emit structured output" and hides the real cause.

Moving the if (!isSuccess) { … } block above the if (hasJsonSchema) block fixes it.

core.error(
"Claude result reported subtype success with is_error:true (run did not complete successfully)",
);
}
if ("errors" in resultMessage && resultMessage.errors) {
core.error(`Execution failed: ${resultMessage.errors.join(", ")}`);
}
throw new Error(
`Claude execution failed: ${
"errors" in resultMessage && resultMessage.errors
? resultMessage.errors.join(", ")
: "unknown error"
resultMessage.subtype === "success" && resultMessage.is_error
? "result is_error:true"
: "errors" in resultMessage && resultMessage.errors
? resultMessage.errors.join(", ")
: "unknown error"
}`,
);
}
Expand Down
82 changes: 79 additions & 3 deletions base-action/src/workload-identity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
*/

import * as core from "@actions/core";
import { mkdirSync, writeFileSync } from "fs";
import { createHash } from "crypto";
import { mkdirSync, rmSync, writeFileSync } from "fs";
import { join } from "path";
import { retryWithBackoff } from "./retry";

Expand Down Expand Up @@ -50,14 +51,72 @@ async function fetchIdentityToken(audience: string) {
return retryWithBackoff(() => core.getIDToken(audience));
}

/**
* Writes a profile config that switches federation resolution to the
* file-backed path. Resolving federation through a profile (rather than bare
* env vars) enables the SDK's on-disk credentials cache, so the several
* `claude` processes the action spawns (plugin installs, main query) share
* one exchanged access token instead of each re-exchanging the single-use
* GitHub OIDC token, which fails with 401 (`jti_reused`).
*
* The profile is intentionally minimal: the SDK gap-fills the federation
* fields (rule, organization, identity-token file, service account, base URL)
* from the ANTHROPIC_* env vars the action already exports, so the file only
* needs to exist to turn the cache on.
*
* The config dir name embeds a fingerprint of the federation inputs. The
* SDK's cache reuses a token on `expires_at` alone, with no record of the
* config that minted it, and the token's scope is bound at mint time — so a
* later action step in the same job (RUNNER_TEMP is per-job) with different
* federation inputs must land in a different dir or it would silently reuse
* the first step's token.
*
* Sharing the cache is only safe while the action spawns its `claude`
* subprocesses sequentially: the SDK cache is not cross-process serialized,
* and concurrent cache misses would each re-exchange the same single-use
* identity token. Parallelizing the plugin installs would reintroduce the
* `jti_reused` failures.
*/
function writeFederationProfile(baseDir: string): string {
// Every input that changes which credential the exchange mints must be in
// here; service_account_id and scope are sent in the exchange request body.

@claude claude Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The fingerprint omits ANTHROPIC_OIDC_AUDIENCE, which contradicts this comment's own invariant. It's a first-class input (action.yml:375) and is used to mint the JWT at line 139.

Two invocations in one job with identical rule/org/service-account/workspace/scope but different audiences land in the same config-<fp> dir. Per the doc comment above, the SDK's cache reuses on expires_at alone — so the second invocation silently reuses the first one's token instead of exchanging with the new audience assertion, bypassing any audience constraint the federation rule enforces.

Suggested change
// here; service_account_id and scope are sent in the exchange request body.
// Every input that changes which credential the exchange mints must be in
// here; service_account_id and scope are sent in the exchange request body,
// and the audience is bound into the identity token assertion.

…plus process.env.ANTHROPIC_OIDC_AUDIENCE?.trim() ?? "" in the hashed array.

const fingerprint = createHash("sha256")
.update(
JSON.stringify([
process.env.ANTHROPIC_FEDERATION_RULE_ID?.trim() ?? "",
process.env.ANTHROPIC_ORGANIZATION_ID?.trim() ?? "",
process.env.ANTHROPIC_SERVICE_ACCOUNT_ID?.trim() ?? "",
process.env.ANTHROPIC_WORKSPACE_ID?.trim() ?? "",
process.env.ANTHROPIC_BASE_URL?.trim() ?? "",
process.env.ANTHROPIC_SCOPE?.trim() ?? "",
]),
)
.digest("hex")
.slice(0, 16);
const configDir = join(baseDir, `config-${fingerprint}`);

mkdirSync(join(configDir, "configs"), { recursive: true, mode: 0o700 });
writeFileSync(
join(configDir, "configs", "default.json"),
JSON.stringify(
{ version: "1.0", authentication: { type: "oidc_federation" } },
null,
2,
),
{ mode: 0o600 },
);
return configDir;
}

/**
* Fetches a GitHub Actions OIDC token, writes it to a file in RUNNER_TEMP,
* exports ANTHROPIC_IDENTITY_TOKEN_FILE, and starts a background refresh so
* the file stays valid for long executions.
*
* Returns undefined when federation is not configured or is shadowed by a
* higher-precedence credential. Callers must invoke stop() when execution
* finishes.
* finishes; it also deletes the identity token and any cached exchanged
* credential.
*/
export async function setupWorkloadIdentity(): Promise<
WorkloadIdentityHandle | undefined
Expand Down Expand Up @@ -101,6 +160,17 @@ export async function setupWorkloadIdentity(): Promise<
}

process.env.ANTHROPIC_IDENTITY_TOKEN_FILE = tokenFile;
if (
process.env.ANTHROPIC_CONFIG_DIR?.trim() ||
process.env.ANTHROPIC_PROFILE?.trim()
) {
core.warning(
"ANTHROPIC_CONFIG_DIR or ANTHROPIC_PROFILE is already set, so the action will not write its own federation profile. Credential caching across the spawned Claude processes follows the existing profile configuration.",
);
} else {
process.env.ANTHROPIC_CONFIG_DIR = writeFederationProfile(tokenDir);
process.env.ANTHROPIC_PROFILE = "default";
}
console.log(
`Workload identity federation configured (rule: ${process.env.ANTHROPIC_FEDERATION_RULE_ID}, identity token file: ${tokenFile})`,
);
Expand All @@ -115,6 +185,12 @@ export async function setupWorkloadIdentity(): Promise<

return {
tokenFile,
stop: () => clearInterval(refreshInterval),
stop: () => {
clearInterval(refreshInterval);

@claude claude Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

An in-flight refresh can recreate the directory right after rmSync deletes it, defeating the cleanup this PR adds.

clearInterval stops future ticks but can't cancel a writeIdentityToken() already awaiting fetchIdentityToken, which goes through retryWithBackoff (3 attempts, 5s → 10s) and can stay pending 15+ seconds. When it finally resolves it runs mkdirSync(tokenDir, …) + writeFileSync(tokenFile, …) (lines 149-150) and writes a fresh OIDC JWT to disk after cleanup. Since RUNNER_TEMP is per-job, it then survives for the rest of the job — exactly what the comment below says the rmSync prevents.

Suggest a stopped flag checked after the await:

let stopped = false;

const writeIdentityToken = async () => {
  const identityToken = await fetchIdentityToken(audience);
  if (stopped) return; // cleanup already ran; don't resurrect the token file
  core.setSecret(identityToken);
  mkdirSync(tokenDir, { recursive: true, mode: 0o700 });
  writeFileSync(tokenFile, identityToken, { mode: 0o600 });
};

…and set stopped = true before clearInterval here.

// RUNNER_TEMP is per-job, not per-step: remove the identity token, the
// profile, and the cached exchanged credential so they don't outlive
// this step.
rmSync(tokenDir, { recursive: true, force: true });

@claude claude Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

stop() can now throw, and it's the first unguarded statement in run.ts's cleanup finally (src/entrypoints/run.ts:371).

force: true only swallows ENOENT — EPERM/EACCES/EBUSY/ENOTEMPTY still throw, and maxRetries defaults to 0. ENOTEMPTY is plausible here specifically because ANTHROPIC_CONFIG_DIR is a subdirectory of tokenDir, so a claude subprocess that outlives the parent can repopulate it mid-walk.

If it throws, the rest of the finally never runs: the tracking comment stays on "Claude is working…", no step summary, and core.setOutput("github_token", …) (run.ts:412) never fires — so the always() "Revoke app token" step, which is gated on steps.run.outputs.github_token != '' (action.yml:483), is skipped and the GitHub App installation token stays valid for up to an hour.

Suggested change
rmSync(tokenDir, { recursive: true, force: true });
try {
rmSync(tokenDir, {
recursive: true,
force: true,
maxRetries: 3,
retryDelay: 100,
});
} catch (error) {
core.warning(
`Failed to remove the workload identity token dir: ${error instanceof Error ? error.message : String(error)}`,
);
}

},
};
}
43 changes: 37 additions & 6 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 @@ -106,7 +106,8 @@ describe("parseSdkOptions", () => {
const result = parseSdkOptions(options);

expect(result.sdkOptions.extraArgs?.["allowedTools"]).toBeUndefined();
expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-3-5-sonnet");
expect(result.sdkOptions.extraArgs?.["model"]).toBeUndefined();
expect(result.sdkOptions.model).toBe("claude-3-5-sonnet");
});

test("should handle hyphenated --allowed-tools flag", () => {
Expand Down Expand Up @@ -366,7 +367,8 @@ describe("parseSdkOptions", () => {
);
expect(mcpConfig.mcpServers).toHaveProperty("server1");
expect(mcpConfig.mcpServers).toHaveProperty("server2");
expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-3-5-sonnet");
expect(result.sdkOptions.extraArgs?.["model"]).toBeUndefined();
expect(result.sdkOptions.model).toBe("claude-3-5-sonnet");
});

test("should handle real-world scenario: action config + user config", () => {
Expand Down Expand Up @@ -436,7 +438,8 @@ describe("parseSdkOptions", () => {
const result = parseSdkOptions(options);

expect(result.sdkOptions.additionalDirectories).toEqual(["/path/to/dir"]);
expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-3-5-sonnet");
expect(result.sdkOptions.extraArgs?.["model"]).toBeUndefined();
expect(result.sdkOptions.model).toBe("claude-3-5-sonnet");
expect(result.sdkOptions.extraArgs?.["add-dir"]).toBeUndefined();
});
});
Expand Down Expand Up @@ -464,7 +467,8 @@ describe("parseSdkOptions", () => {

const result = parseSdkOptions(options);

expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-haiku");
expect(result.sdkOptions.extraArgs?.["model"]).toBeUndefined();
expect(result.sdkOptions.model).toBe("claude-haiku");
expect(result.sdkOptions.allowedTools).toEqual(["Edit"]);
});

Expand All @@ -475,7 +479,8 @@ describe("parseSdkOptions", () => {

const result = parseSdkOptions(options);

expect(result.sdkOptions.extraArgs?.["model"]).toBe("claude-haiku");
expect(result.sdkOptions.extraArgs?.["model"]).toBeUndefined();
expect(result.sdkOptions.model).toBe("claude-haiku");
});

test("should not strip inline # that appears inside a quoted value", () => {
Expand All @@ -485,11 +490,37 @@ describe("parseSdkOptions", () => {

const result = parseSdkOptions(options);

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

describe("model handling", () => {
test("should map --model from claudeArgs to sdkOptions.model", () => {
const options: ClaudeOptions = {
claudeArgs: "--model claude-haiku-4-5-20251001",
};

const result = parseSdkOptions(options);

expect(result.sdkOptions.model).toBe("claude-haiku-4-5-20251001");
expect(result.sdkOptions.extraArgs?.["model"]).toBeUndefined();
});

test("should prefer direct model option over --model from claudeArgs", () => {
const options: ClaudeOptions = {
model: "claude-sonnet-4-6",
claudeArgs: "--model claude-haiku-4-5-20251001",
};

const result = parseSdkOptions(options);

expect(result.sdkOptions.model).toBe("claude-sonnet-4-6");
expect(result.sdkOptions.extraArgs?.["model"]).toBeUndefined();
});
});

describe("environment variables passthrough", () => {
test("should include OTEL environment variables in sdkOptions.env", () => {
// Set up test environment variables
Expand Down
Loading
Loading