fix(sandbox): scrub sensitive credentials from sandbox environment#660
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughSandbox environment construction now removes sensitive variables, propagates workspace-root handling, and denies access to detected credential paths. Atomic file writes retry Windows sharing violations across persistence flows. A context budget test now uses a 20,000-token window with expanded diagnostics. ChangesWindows rename resilience
Sandbox environment hardening
Context budget test update
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SandboxPlan
participant sandboxEnvironmentForCommand
participant scrubSensitiveEnv
participant PermissionProfile
SandboxPlan->>sandboxEnvironmentForCommand: Build environment with workspaceRoot
sandboxEnvironmentForCommand->>scrubSensitiveEnv: Remove sensitive environment keys
SandboxPlan->>PermissionProfile: Add discovered credential deny-read paths
PermissionProfile-->>SandboxPlan: Return sandbox permissions
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/cron/store.go (1)
136-150: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winOrphaned
.tmpfile on permanent rename failure.If
renameWithRetryexhausts all attempts and returns an error,metadata.json.tmpis left on disk —writeJobreturns the error without removingtmp. Sibling implementations in this same PR (sessions/store.go'swriteMetadata,windows_unelevated.go'srecordWindowsUnelevatedAppliedPlan) both clean up with_ = os.Remove(tmp)on failure; this one doesn't.🧹 Proposed fix
tmp := filepath.Join(dir, "metadata.json.tmp") if err := os.WriteFile(tmp, data, 0o600); err != nil { return err } - return renameWithRetry(tmp, filepath.Join(dir, "metadata.json")) + if err := renameWithRetry(tmp, filepath.Join(dir, "metadata.json")); err != nil { + _ = os.Remove(tmp) + return err + } + return nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cron/store.go` around lines 136 - 150, Update writeJob to remove the temporary metadata file when renameWithRetry fails, using _ = os.Remove(tmp) before returning the rename error; preserve the existing successful rename path and earlier error handling.
🧹 Nitpick comments (2)
internal/sandbox/runner_test.go (1)
612-638: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTest coverage gaps: missing case-insensitivity and malformed-entry cases.
The test only covers 7 of 18 sensitive keys and uses exact casing, so it wouldn't catch a regression in the
strings.EqualFoldlogic. It also doesn't verify that malformed entries (no=) are retained, which is an explicit code path inscrubSensitiveEnv.🧪 Proposed test enhancements
func TestScrubSensitiveEnv(t *testing.T) { inputEnv := []string{ "PATH=/usr/bin", "OPENAI_API_KEY=sk-proj-12345", "ANTHROPIC_API_KEY=sk-ant-12345", "GEMINI_API_KEY=AIzaSy12345", "DEEPSEEK_API_KEY=ds-12345", "GITHUB_TOKEN=ghp_12345", "AWS_ACCESS_KEY_ID=AKIA12345", "AWS_SECRET_ACCESS_KEY=secret12345", + "AWS_SESSION_TOKEN=session12345", + "GROQ_API_KEY=groq-12345", + "COHERE_API_KEY=cohere-12345", + "MISTRAL_API_KEY=mistral-12345", + "PERPLEXITY_API_KEY=pplx-12345", + "OPENROUTER_API_KEY=or-12345", + "ANYSCALE_API_KEY=anyscale-12345", + "TOGETHER_API_KEY=together-12345", + "AZURE_OPENAI_API_KEY=azure-12345", + "GITLAB_TOKEN=glpat-12345", + "GH_TOKEN=ghp_67890", + "openai_api_key=lowercase-should-also-be-scrubbed", + "NO_EQUALS_HERE", "SAFE_VAR=hello", } scrubbed := scrubSensitiveEnv(inputEnv) expectedMap := map[string]bool{ - "PATH": true, - "SAFE_VAR": true, + "PATH": true, + "SAFE_VAR": true, + "NO_EQUALS_HERE": true, } for _, entry := range scrubbed { key, _, _ := strings.Cut(entry, "=") if !expectedMap[key] { t.Errorf("found sensitive/unexpected environment variable: %s", entry) } } - if len(scrubbed) != 2 { - t.Errorf("expected 2 environment variables, got %d: %v", len(scrubbed), scrubbed) + if len(scrubbed) != 3 { + t.Errorf("expected 3 environment variables, got %d: %v", len(scrubbed), scrubbed) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/sandbox/runner_test.go` around lines 612 - 638, Expand TestScrubSensitiveEnv to include sensitive environment keys with mixed casing, covering the case-insensitive behavior of scrubSensitiveEnv and all relevant sensitive-key categories. Add malformed entries without “=” and assert they are retained, while continuing to verify sensitive entries are removed and safe entries remain.Source: Linters/SAST tools
internal/cron/store.go (1)
327-344: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRetry budget is only ~90ms total (10 attempts × 10ms sleep).
Real-world Windows sharing violations from AV scanners or indexers can outlast this window, in which case the operation still fails permanently, just ~90ms later. Worth considering more attempts and/or a small backoff (this is speculative on production timing, hence flagged for verification rather than as a hard requirement).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cron/store.go` around lines 327 - 344, Consider increasing the retry budget in renameWithRetry for Windows permission and sharing-violation errors, using more attempts and/or a modest backoff so transient file locks lasting beyond the current ~90ms window can recover. Preserve the existing immediate return for successful renames and non-retryable errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/cron/store.go`:
- Around line 327-353: Extract the duplicated rename retry logic into a shared
internal filesystem utility, including the injectable rename function and
Windows sharing-violation detection. Update renameWithRetry and the equivalent
implementations in sessions/store.go, windows_unelevated.go, and mailbox.go to
delegate to that helper while preserving the existing retry count, delay, and
non-retryable error behavior; add or migrate tests so all call sites can
exercise injected rename failures.
In `@internal/sandbox/runner.go`:
- Around line 968-1009: Update scrubSensitiveEnv to cover every credential name
in the provider catalog, including GOOGLE_API_KEY, OLLAMA_API_KEY,
HUGGINGFACE_API_KEY, LONGCAT_API_KEY, XAI_API_KEY, and VENICE_API_KEY. Prefer
deriving the sensitiveKeys entries from the existing provider catalog or shared
credential definition so future providers remain synchronized, while preserving
the current case-insensitive filtering behavior.
- Line 385: Restrict scrubbing to inherited environment variables by moving the
scrubSensitiveEnv call in the runner flow into the specEnv == nil branch, and
apply the same change in the Windows runner. Preserve specEnv values unchanged
so command-supplied credentials such as OPENAI_API_KEY can pass through.
---
Outside diff comments:
In `@internal/cron/store.go`:
- Around line 136-150: Update writeJob to remove the temporary metadata file
when renameWithRetry fails, using _ = os.Remove(tmp) before returning the rename
error; preserve the existing successful rename path and earlier error handling.
---
Nitpick comments:
In `@internal/cron/store.go`:
- Around line 327-344: Consider increasing the retry budget in renameWithRetry
for Windows permission and sharing-violation errors, using more attempts and/or
a modest backoff so transient file locks lasting beyond the current ~90ms window
can recover. Preserve the existing immediate return for successful renames and
non-retryable errors.
In `@internal/sandbox/runner_test.go`:
- Around line 612-638: Expand TestScrubSensitiveEnv to include sensitive
environment keys with mixed casing, covering the case-insensitive behavior of
scrubSensitiveEnv and all relevant sensitive-key categories. Add malformed
entries without “=” and assert they are retained, while continuing to verify
sensitive entries are removed and safe entries remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 11a0ba9d-8b49-479b-9d6a-5d964a31cb3a
📒 Files selected for processing (9)
internal/contextreport/contextreport_test.gointernal/cron/store.gointernal/sandbox/runner.gointernal/sandbox/runner_test.gointernal/sandbox/windows_runner.gointernal/sandbox/windows_unelevated.gointernal/sessions/store.gointernal/swarm/mailbox.gointernal/swarm/mailbox_test.go
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/sandbox/runner.go`:
- Around line 403-409: Update the Windows environment override condition in the
plan-building flow to use the selected Windows backend or target platform
instead of runtime.GOOS. Ensure COMSPEC, SystemRoot, and WINDIR are included
whenever a Windows plan is being built, including on non-Windows hosts, while
preserving the existing envListValue defaults.
- Around line 373-375: Update sandboxEnvironment to name and forward its third
argument as workspaceRoot when calling sandboxEnvironmentForCommand, replacing
the hardcoded empty string so callers receive HOME set to the provided workspace
root.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a285ba6a-868c-4dc4-8f60-080aae6337a1
📒 Files selected for processing (3)
internal/sandbox/reentrancy_test.gointernal/sandbox/runner.gointernal/sandbox/windows_runner.go
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Pushed cab3dd7 to fix the Smoke and Zero Review failures: removed a stray blank line at EOF in runner_test.go and mailbox_test.go and a doubled blank line in windows_runner.go, all flagged by gofmt and diff hygiene. The CodeRabbit comments will be addressed separately. |
|
Pushed d96e053 addressing the CodeRabbit review:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/sandbox/runner.go`:
- Around line 985-994: Update the sensitive environment-variable handling around
the sensitiveKeys construction to scrub AWS_PROFILE and
GOOGLE_APPLICATION_CREDENTIALS instead of exempting them. Ensure the sandbox
cannot use these variables to access ~/.aws/credentials or the service-account
JSON credential file, while preserving unrelated environment-variable behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2ccd38fa-4f9d-4850-bdd5-cfafcf4e37ab
📒 Files selected for processing (7)
internal/cron/store.gointernal/fsutil/rename.gointernal/fsutil/rename_test.gointernal/sandbox/runner.gointernal/sandbox/windows_unelevated.gointernal/sessions/store.gointernal/swarm/mailbox.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/sandbox/windows_unelevated.go
|
The macOS smoke failure on 47ef254 was the sandbox policy JSON golden test: the new default credential deny-read entries include whatever stores exist in the real home, and the macOS CI image ships ~/.aws, so host paths leaked into the golden comparison. Pushed 614918d pinning HOME to an empty temp directory in that test so it stays hermetic. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/fsutil/rename.go (1)
26-31: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider also retrying
ERROR_LOCK_VIOLATION(errno 33).
ERROR_LOCK_VIOLATIONis another transient Windows rename failure (a byte-range lock on the destination) thatMoveFileExcan return. It's distinct from errno 32 (sharing violation) and errno 5 (access denied, already caught byos.IsPermission). Without retrying it, a transient lock causes an immediate write failure and temp-file cleanup, losing the atomic write.♻️ Proposed addition
if runtime.GOOS == "windows" { - if os.IsPermission(err) || isWindowsSharingViolation(err) { + if os.IsPermission(err) || isWindowsSharingViolation(err) || isWindowsLockViolation(err) { time.Sleep(10 * time.Millisecond) continue } }And add the helper:
+func isWindowsLockViolation(err error) bool { + var errno syscall.Errno + if errors.As(err, &errno) { + const ERROR_LOCK_VIOLATION syscall.Errno = 33 + return errno == ERROR_LOCK_VIOLATION + } + return false +}Please verify that
MoveFileEx(used byos.Renameon Windows) can indeed returnERROR_LOCK_VIOLATIONfor the atomic-rename use cases in this codebase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/fsutil/rename.go` around lines 26 - 31, Update the Windows retry condition in the rename retry loop to also recognize ERROR_LOCK_VIOLATION (errno 33), using a focused helper alongside isWindowsSharingViolation. Verify that os.Rename’s MoveFileEx path can return this error for the atomic rename cases used here, and preserve the existing retry behavior for permission and sharing violations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/sandbox/profile.go`:
- Around line 138-161: Update credentialDenyReadPaths and
credentialDenyReadPathsIn so an unresolved or blank home directory skips only
home-based candidates while still processing a nonblank
GOOGLE_APPLICATION_CREDENTIALS target. Remove the early returns that prevent
Google target handling, conditionally append the .aws, gcloud, and .azure paths,
and update the blank-home test to expect the supplied Google credential path.
---
Nitpick comments:
In `@internal/fsutil/rename.go`:
- Around line 26-31: Update the Windows retry condition in the rename retry loop
to also recognize ERROR_LOCK_VIOLATION (errno 33), using a focused helper
alongside isWindowsSharingViolation. Verify that os.Rename’s MoveFileEx path can
return this error for the atomic rename cases used here, and preserve the
existing retry behavior for permission and sharing violations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a0bbcbe5-0a97-4015-bc8a-a3f521eb9e5b
📒 Files selected for processing (15)
internal/cli/sandbox_test.gointernal/contextreport/contextreport_test.gointernal/cron/store.gointernal/fsutil/rename.gointernal/fsutil/rename_test.gointernal/sandbox/manager_test.gointernal/sandbox/profile.gointernal/sandbox/reentrancy_test.gointernal/sandbox/runner.gointernal/sandbox/runner_test.gointernal/sandbox/windows_runner.gointernal/sandbox/windows_unelevated.gointernal/sessions/store.gointernal/swarm/mailbox.gointernal/swarm/mailbox_test.go
There was a problem hiding this comment.
I read the scrub end to end. The sensitive set comes from the provider catalog plus a hand-listed cloud/VCS list, scrubbing runs before the child env is assembled, and the result is wired as the child's explicit environ (command.Env = plan.Env), so a scrubbed var isn't recoverable through /proc or any inherited copy. PATH, HOME, COMSPEC/SystemRoot/WINDIR, and proxy settings are preserved, and the new HOME=workspaceRoot override is gated to the Windows backend so Linux/macOS behavior is unchanged. Build, vet, gofmt, and the sandbox/fsutil/swarm tests pass and the tree merges clean.
One gap worth a follow-up before this is fully airtight: ZERO_WEBSEARCH_API_KEY (internal/tools/web_search.go:201) is a real API key Zero reads from env but isn't in the scrub list I'd add it (and optionally ZERO_DAEMON_REMOTE_TOKEN) to the sensitive set. Otherwise approve.
|
@gnanam1990 when you have time this is the one in the sandbox cluster I'd value your independent read on. The scrub mechanism is sound (env is assembled then scrubbed before the child exec, and wired as the explicit environ so nothing leaks via /proc or an inherited copy), and the filesystem deny-read covers ~/.aws / gcloud / azure. The one gap I flagged: ZERO_WEBSEARCH_API_KEY (internal/tools/web_search.go:201, used as the web-search bearer key) isn't in the scrub set it's a real secret read from env, so if a user sets it in their shell a sandboxed command could still see it. Trivial fix; I left it as a comment rather than blocking. |
…Windows env on backend
…st from provider catalog Address CodeRabbit review feedback: - Add internal/fsutil.RenameWithRetry with an injectable rename override and move the four duplicated renameWithRetry/isWindowsSharingViolation copies (cron, sessions, sandbox, swarm) onto it. Mailbox keeps its rename test seam by passing its field through. Add fsutil tests for the Windows retry and non-retryable paths. - Remove the orphaned metadata.json.tmp in cron writeJob when the rename permanently fails, matching the sibling writers. - Derive scrubSensitiveEnv keys from providercatalog AuthEnvVars plus a static list of non-catalog secrets, so new providers are scrubbed automatically. AWS_PROFILE and GOOGLE_APPLICATION_CREDENTIALS stay because they are pointers, not secrets.
Address CodeRabbit finding that scrubbed env pointers still left the underlying credential files readable under the read-all workspace posture: - Add credentialDenyReadPaths: default profile-level deny-read entries for ~/.aws, ~/.config/gcloud, ~/.azure, and the GOOGLE_APPLICATION_CREDENTIALS target file, wired into both permission profile builders so the seatbelt and Linux helper backends enforce them. Entries are stat-filtered and a store nested under a user AllowRead entry is dropped, keeping an explicit opt-out. - Keep the defaults out of Policy.DenyRead, whose emptiness gates escalated execution, and skip Windows so the profile deny list does not flip the runner off the WRITE_RESTRICTED token path. - Scrub GOOGLE_APPLICATION_CREDENTIALS from the sandbox env; keep AWS_PROFILE since the deny-read rule on ~/.aws is the real protection and the SDKs fall back to the default profile regardless. - Add credential deny path tests, extend the scrub test, relax the profile test's exact deny count to containment.
The default credential-store deny-read entries depend on what exists in the real home directory (the macOS CI image ships ~/.aws), which leaked host paths into the policy JSON golden comparison. Point HOME at an empty temp directory so the golden stays hermetic.
…retry lock violations Address CodeRabbit review feedback: - credentialDenyReadPaths no longer returns early when the home lookup fails; home-based candidates become conditional and the GOOGLE_APPLICATION_CREDENTIALS target is always considered. - fsutil.RenameWithRetry also retries ERROR_LOCK_VIOLATION (errno 33), matching the mcp lock handling; helper renamed to isWindowsSharingOrLockViolation. - Extend tests: homeless GOOGLE_APPLICATION_CREDENTIALS case and a lock violation step in the retry sequence.
euxaristia
force-pushed
the
fix/sandbox-credential-leakage
branch
from
e86d6f6 to
6960034
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/sandbox/runner_test.go`:
- Around line 659-671: Update the assertions in the scrubbed-environment test
around expectedMap and scrubbed to compare the complete expected key/value set,
including PATH, SAFE_VAR, and AWS_PROFILE values, rather than only allowing keys
and checking the count. Reject duplicate keys and ensure no expected entry is
missing, while preserving the existing sensitive-variable validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6d53d251-3be0-4247-8ffa-94e8eb6839fe
📒 Files selected for processing (15)
internal/cli/sandbox_test.gointernal/contextreport/contextreport_test.gointernal/cron/store.gointernal/fsutil/rename.gointernal/fsutil/rename_test.gointernal/sandbox/manager_test.gointernal/sandbox/profile.gointernal/sandbox/reentrancy_test.gointernal/sandbox/runner.gointernal/sandbox/runner_test.gointernal/sandbox/windows_runner.gointernal/sandbox/windows_unelevated.gointernal/sessions/store.gointernal/swarm/mailbox.gointernal/swarm/mailbox_test.go
🚧 Files skipped from review as they are similar to previous changes (14)
- internal/contextreport/contextreport_test.go
- internal/fsutil/rename_test.go
- internal/cli/sandbox_test.go
- internal/cron/store.go
- internal/sandbox/windows_unelevated.go
- internal/fsutil/rename.go
- internal/sandbox/reentrancy_test.go
- internal/sandbox/profile.go
- internal/sandbox/windows_runner.go
- internal/swarm/mailbox_test.go
- internal/sandbox/runner.go
- internal/swarm/mailbox.go
- internal/sandbox/manager_test.go
- internal/sessions/store.go
… keys TestScrubSensitiveEnv only checked an allowlist plus a length count, so a duplicate allowed key silently replacing a required one would still pass. Compare the exact expected key/value map instead. Also add ZERO_WEBSEARCH_API_KEY and ZERO_DAEMON_REMOTE_TOKEN to scrubSensitiveEnv's sensitiveKeys list: both are real bearer secrets read from the environment but were missing from the scrub list, leaving them readable by sandboxed commands when set in the parent shell.
|
Pushed 570b288: tightened TestScrubSensitiveEnv to compare the exact expected key/value map instead of an allowlist + count (which would have passed even if a required key were silently dropped in favor of a duplicate), and added ZERO_WEBSEARCH_API_KEY and ZERO_DAEMON_REMOTE_TOKEN to the sensitiveKeys scrub list — both are real secrets read via os.Getenv that were missing from it. |
3 participants
Problem
The agent processes (which hold sensitive environment variables like OPENAI_API_KEY, ANTHROPIC_API_KEY, GEMINI_API_KEY, DEEPSEEK_API_KEY, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and GITHUB_TOKEN) passed their entire environment verbatim to sandboxed user command executions when no custom environment was specified. This could leak credentials to sandboxed subprocesses.
Solution
Summary by CodeRabbit
Bug Fixes
Tests