Skip to content

fix: filter LLM API keys from bash tool child env#128

Open
Darkroom4364 wants to merge 1 commit intoPwnKit-Labs:mainfrom
Darkroom4364:fix/api-key-filtering
Open

fix: filter LLM API keys from bash tool child env#128
Darkroom4364 wants to merge 1 commit intoPwnKit-Labs:mainfrom
Darkroom4364:fix/api-key-filtering

Conversation

@Darkroom4364
Copy link
Copy Markdown
Contributor

  • Adds sanitizedEnv() to strip sensitive keys before passing to execSync
  • Keeps PATH, HOME, TARGET, and auth env vars

Copy link
Copy Markdown
Contributor Author

@Darkroom4364 Darkroom4364 left a comment

Choose a reason for hiding this comment

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

Good change, a few things:

Bug — missed call site (line ~1496)
The interactive shell session (create action) builds its env as { TARGET: …, ...this.buildAuthEnvVars() } — no sanitizedEnv(), so it inherits process.env via Node defaults and leaks every key this PR tries to strip.

Missing patterns

  • MISTRAL_API, COHERE_API, HUGGINGFACE, DEEPSEEK_API, TOGETHER_API, GROQ_API — all common in the wild.
  • _KEY / _SECRET suffix catch-all would be more future-proof than an allowlist of prefixes.

Nit
sanitizedEnv() rebuilds the filtered object on every call. Fine for now, but a module-level lazy cache would be free and cleaner.

Overall: scope is right (only tools.ts), diff is minimal. Fix the second call site and widen the patterns, then LGTM.

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