Uh oh!
There was an error while loading. Please reload this page.
fix: validate provider name in invalidate_cloud_key and improve key validation - #1017
Merged
Merged
Conversation
…alidation
- Add regex validation (^[a-z0-9][a-z0-9._-]{0,63}$) to invalidate_cloud_key()
in shared/key-request.sh to prevent path traversal attacks that could delete
arbitrary files via crafted provider names (e.g., ../../etc/important)
- Improve validKeyVal() in key-server.ts to block control characters
(U+0000-U+001F, U+007F-U+009F) and enforce a 4096-byte max length on
API key values, preventing injection of null bytes, newlines, and
excessively long values
Agent: security-auditor
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>louisgv
approved these changes
Feb 13, 2026
louisgv
left a comment
Collaborator
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Findings
No security issues found. This PR is a security improvement:
shared/key-request.sh:205-210— Adds path traversal prevention ininvalidate_cloud_key(). Provider name validated against^[a-z0-9][a-z0-9._-]{0,63}$, preventing../directory escape. Clean and correct..claude/skills/setup-agent-team/key-server.ts:304-308— StrengthensvalidKeyVal()with length bounds (0-4096) and control character blocking (\x00-\x1f,\x7f-\x9f). Good defense-in-depth.
Tests
- bash -n: PASS (
shared/key-request.sh) - bun test: PASS (7655 pass, 184 pre-existing failures unrelated to this PR)
- curl|bash pattern: OK (no changes to source/eval fallback)
- macOS compat: OK (no bash 3.x incompatibilities introduced)
-- security/pr-reviewer
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Path traversal fix (
shared/key-request.sh:202-211):invalidate_cloud_key()now validates theproviderparameter against^[a-z0-9][a-z0-9._-]{0,63}$before constructing a file path, preventing path traversal attacks that could delete arbitrary files (e.g.,../../etc/passwd). This matches the sameSAFE_PROVIDER_REregex already used inkey-server.ts.Key validation hardening (
.claude/skills/setup-agent-team/key-server.ts:303):validKeyVal()now blocks control characters (U+0000-U+001F, U+007F-U+009F) and enforces a 4096-byte max length. Previously it only blocked shell metacharacters, allowing null bytes, newlines, and arbitrarily long values through.Fixes#1013
Test plan
bash -n shared/key-request.shpassesbun testpasses (7654 pass, 185 pre-existing failures unrelated to these changes)invalidate_cloud_key "../../../tmp/test"returns 1 and logs errorvalidKeyVal("key-with-\x00-null")returns falsevalidKeyVal("")returns false (empty)validKeyVal("sk-valid-key-12345")returns true-- refactor/security-auditor