Skip to content

Auto-set subprocess env scrub when allowed_non_write_users is configured#1093

Open
OctavianGuzu wants to merge 3 commits intomainfrom
oct/subprocess-env-scrub-v2
Open

Auto-set subprocess env scrub when allowed_non_write_users is configured#1093
OctavianGuzu wants to merge 3 commits intomainfrom
oct/subprocess-env-scrub-v2

Conversation

@OctavianGuzu
Copy link
Collaborator

@OctavianGuzu OctavianGuzu commented Mar 20, 2026

Sets CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1 automatically whenever allowed_non_write_users is non-empty. The CLI (v2.1.79+) reads this flag to do a best-effort scrub of Anthropic, cloud, and GitHub Actions secrets from the environment of subprocesses it spawns (Bash tool, hooks, MCP stdio, etc.). The parent process keeps the vars for its own API calls; only child subprocess environments are scrubbed.

Updates the input description and docs/security.md to note the behavior.

Downstream workflows that set allowed_non_write_users get this automatically — no workflow-file changes needed.

…s 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
@OctavianGuzu OctavianGuzu marked this pull request as ready for review March 20, 2026 10:44
Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

LGTM — straightforward security hardening that auto-enables subprocess env scrubbing when allowed_non_write_users is set.

Extended reasoning...

Overview

This PR adds a single env var (CLAUDE_CODE_SUBPROCESS_ENV_SCRUB) to the step env block in action.yml, set to "1" when allowed_non_write_users is non-empty. It also updates the allowed_non_write_users input description and docs/security.md to document the new behavior. Two files changed, minimal diff.

Security risks

This change improves the security posture by automatically scrubbing secrets from subprocess environments when untrusted users can trigger the action. No new attack surface is introduced.

Level of scrutiny

Low scrutiny warranted. This is a small, self-contained config change that follows the exact same pattern used by CLAUDE_CODE_USE_BEDROCK, CLAUDE_CODE_USE_VERTEX, and CLAUDE_CODE_USE_FOUNDRY in the same env block. The documentation updates accurately describe the behavior.

Other factors

The nit bug report about step-level env overriding user-set values is a valid forward-looking design concern, but has near-zero practical impact: CLAUDE_CODE_SUBPROCESS_ENV_SCRUB is brand new (introduced in CLI v2.1.79), so no existing workflows could be relying on setting it at a higher scope. The suggested fallback pattern (env.CLAUDE_CODE_SUBPROCESS_ENV_SCRUB || "") is a reasonable improvement but not blocking.

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
Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

LGTM — the env var fallback from my earlier nit has been addressed.

Extended reasoning...

Overview

This PR adds two small changes: (1) a new CLAUDE_CODE_SUBPROCESS_ENV_SCRUB env var in action.yml that auto-enables subprocess environment scrubbing when allowed_non_write_users is set, with a proper fallback to the inherited env value, and (2) updated documentation in both the input description and docs/security.md to describe this behavior.

Security risks

This change improves security posture by automatically scrubbing secrets from subprocess environments when untrusted users can trigger the action. No new attack surface is introduced. The scrubbing is defense-in-depth and the docs appropriately caveat that it "reduces but does not eliminate" prompt injection risk.

Level of scrutiny

This is a low-risk configuration change to a GitHub Action definition and its documentation. The logic is a single GitHub Actions expression following established patterns already used for CLAUDE_CODE_USE_BEDROCK/VERTEX/FOUNDRY in the same file. The fallback to env.CLAUDE_CODE_SUBPROCESS_ENV_SCRUB ensures users can independently opt in even without setting allowed_non_write_users.

Other factors

My previous nit about the env var fallback has been addressed in commit 169838b. The expression now correctly falls back to the inherited environment value when allowed_non_write_users is not set, matching the fix I suggested. No bugs were found by the automated bug hunting system. The change is small, self-contained, and well-documented.

Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

LGTM — the env fallback fix addresses my previous nit, and the docs updates are clear.

Extended reasoning...

Overview

This PR adds two things: (1) auto-setting CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1 when allowed_non_write_users is configured, and (2) updating the input description and docs/security.md to document the behavior and opt-out mechanism. Only two files are touched (action.yml and docs/security.md) with minimal additions.

Security risks

This PR is a net security improvement — it adds defense-in-depth by scrubbing secrets from subprocess environments when processing potentially untrusted input. The opt-out is documented (CLAUDE_CODE_SUBPROCESS_ENV_SCRUB: 0). No new attack surface is introduced.

Level of scrutiny

Low scrutiny warranted. The code change is a single line in action.yml adding an env var expression, plus documentation updates. The expression logic is straightforward GitHub Actions syntax. My previous nit about the env var override was addressed — the current expression correctly falls back to any user-set value first before auto-setting based on allowed_non_write_users.

Other factors

The inline comment from my prior review was resolved, confirming the author addressed the feedback. No bugs were found by the automated bug hunting system. The change is backward-compatible — existing workflows without allowed_non_write_users see no behavior change.

Copy link
Collaborator

@wolffiex wolffiex left a comment

Choose a reason for hiding this comment

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

Please make sure this gets a beefy release note

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.

2 participants