t4954: Add section 8.4 column-level query discipline and URL credential detection#4956
Conversation
…credential detection (#4954) Add layered defense (Option C) against credential exposure from application config tables that store secrets in URL query parameters. Layer 1 - Prompt rule (build.txt section 8.4): - Never fetch raw record values from webhook/integration/OAuth config tables - Query schema/keys first, then extract only non-credential fields - Treat any URL field in application config as potentially containing credentials Layer 2 - Output redaction tooling: - Add credential_exposure category to prompt-guard-helper.sh (inline + YAML) - Detect secrets in URL query params: secret=, token=, api_key=, password=, access_token=, auth=, client_secret=, webhook_secret= - Redact matched values in sanitize function (replaces 8+ char values with [REDACTED]) - 9 new tests (7 detection + 2 sanitization), all passing, zero regressions Closes #4954
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the system's ability to prevent sensitive credential exposure, particularly from application configuration data. It implements a two-pronged approach: first, by educating the system on safer querying practices to avoid fetching raw credential fields, and second, by introducing robust pattern-matching and redaction tools to automatically identify and mask credentials found in URL query parameters within output, thereby bolstering overall security. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedPull request was closed or merged during review WalkthroughAdds credential-exposure detection and redaction: new t4954 patterns for URL query secrets, prompt guidance section 8.4 to query config metadata instead of raw values, and sanitizer updates to redact credential query parameters before outputs enter the conversation. Changes
Sequence DiagramsequenceDiagram
participant Agent as Agent Process
participant Cmd as Command Execution
participant RawOut as Raw Output
participant Guard as prompt-guard-helper (Sanitizer)
participant Conv as Conversation Context
Agent->>Cmd: Execute DB query (e.g., SELECT webhook config)
Cmd->>RawOut: Returns config JSON with URL query params
RawOut->>Guard: Stream contains secret=VALUE or token=VALUE
Guard->>Guard: Match credential_exposure patterns
Guard->>Guard: Redact values → secret=[REDACTED]
Guard->>Conv: Emit sanitized output
Conv->>Agent: Safe transcript for agent reasoning
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Mar 15 20:35:42 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces important safeguards against credential exposure from application configuration. The new prompt rules, detection patterns, and output sanitization create a robust, layered defense. My review focuses on ensuring consistency across these new mechanisms. I've pointed out that the key= parameter, mentioned in the new prompt guidelines, is missing from the detection patterns and sanitization logic. Addressing this will make the protection more comprehensive and consistent with the documentation.
| description: "URL query param: api_key/apikey" | ||
| pattern: '[?&](api_key|apikey|api-key)=[^&\s]{8,}' |
There was a problem hiding this comment.
For consistency with the new rule in .agents/prompts/build.txt (section 8.4, line 295), which mentions ?key=, this pattern should also detect key=. The current pattern only detects api_key, apikey, and api-key. The description should also be updated to reflect this change.
description: "URL query param: key/api_key/apikey"
pattern: '[?&](key|api_key|apikey|api-key)=[^&\s]{8,}'References
- This rule emphasizes the need to include all known variations of sensitive patterns (like
key=) for comprehensive security detection and sanitization. - This rule promotes consistency by ensuring that if a new sensitive parameter (
key=) is identified, all relevant detection and sanitization logic across the codebase is updated.
| LOW|system_prompt_extraction|Prompt leak via translation|([Tt]ranslate|[Cc]onvert)\s+(your\s+)?(system\s+)?(prompt|instructions|rules)\s+(to|into)\s+(French|Spanish|Chinese|another\s+language) | ||
| MEDIUM|credential_exposure|URL query param: secret|[?&]secret=[^&\s]{8,} | ||
| MEDIUM|credential_exposure|URL query param: token|[?&]token=[^&\s]{8,} | ||
| MEDIUM|credential_exposure|URL query param: key|[?&](api_key|apikey|api-key)=[^&\s]{8,} |
There was a problem hiding this comment.
There are a couple of inconsistencies on this line:
- The description is "URL query param: key", but the pattern does not match
key=. - The pattern is missing
key=which is mentioned as a sensitive parameter in.agents/prompts/build.txt.
To ensure all documented credential parameters are detected, please update the pattern and description for consistency.
| MEDIUM|credential_exposure|URL query param: key|[?&](api_key|apikey|api-key)=[^&\s]{8,} | |
| MEDIUM|credential_exposure|URL query param: key/api_key|[?&](key|api_key|apikey|api-key)=[^&\s]{8,} |
References
- This rule emphasizes the need to include all known variations of sensitive patterns (like
key=) for comprehensive security detection and sanitization. - This rule promotes consistency by ensuring that if a new sensitive parameter (
key=) is identified, all relevant detection and sanitization logic across the codebase is updated.
|
|
||
| # Redact credential values in URL query parameters (t4954) | ||
| # Matches ?secret=VALUE or &token=VALUE etc. and replaces VALUE with [REDACTED] | ||
| sanitized=$(printf '%s' "$sanitized" | sed -E 's/([?&](secret|token|api_key|apikey|api-key|password|access_token|auth|authorization|client_secret|webhook_secret)=)[^&[:space:]]{8,}/\1[REDACTED]/g') |
There was a problem hiding this comment.
The sanitization regex is missing the key parameter, which is mentioned as a sensitive parameter in the new section 8.4 of .agents/prompts/build.txt. To ensure consistent redaction of all potential credentials, key should be added to the list of parameters in the regex.
| sanitized=$(printf '%s' "$sanitized" | sed -E 's/([?&](secret|token|api_key|apikey|api-key|password|access_token|auth|authorization|client_secret|webhook_secret)=)[^&[:space:]]{8,}/\1[REDACTED]/g') | |
| sanitized=$(printf '%s' "$sanitized" | sed -E 's/([?&](key|secret|token|api_key|apikey|api-key|password|access_token|auth|authorization|client_secret|webhook_secret)=)[^&[:space:]]{8,}/\1[REDACTED]/g') |
References
- This rule emphasizes the need to include all known variations of sensitive patterns (like
key=) for comprehensive security detection and sanitization. - This rule promotes consistency by ensuring that if a new sensitive parameter (
key=) is identified, all relevant detection and sanitization logic across the codebase is updated.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/prompt-guard-helper.sh:
- Around line 1496-1501: Replace the real-looking credential literals used in
the _test_expect calls (e.g., test descriptions "URL with ?secret= param", "URL
with &token= param", etc.) with clearly fake placeholders that preserve
length/shape for regex testing (for example "REDACTED_SECRET_PLACEHOLDER",
"REDACTED_TOKEN_PLACEHOLDER", "REDACTED_AWS_KEY_PLACEHOLDER",
"REDACTED_PASSWORD_PLACEHOLDER", "REDACTED_JWT_PLACEHOLDER",
"REDACTED_CLIENT_SECRET_PLACEHOLDER"); update the six _test_expect invocations
so the URL query values are non-sensitive placeholders while still exercising
the same patterns and add a brief comment near these _test_expect lines
referencing that real secrets must be stored securely (env/secret manager)
rather than in test literals.
- Line 1502: The test call to _test_expect ("Short param value (no match)") uses
"key=abc" which is not a tracked/detected parameter so the length check isn't
exercised; update the URL argument passed to _test_expect to use one of the
script's tracked parameter names (the same param name used in other tests in
this file) and give it a short value under the threshold (e.g., param=ab) so the
short-value branch in the parameter length validation is actually tested.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8710458e-994f-47f8-ae19-67bf8b5fc3af
📒 Files selected for processing (3)
.agents/configs/prompt-injection-patterns.yaml.agents/prompts/build.txt.agents/scripts/prompt-guard-helper.sh
| _test_expect "URL with ?password= param" 2 "https://service.example.com/auth?password=SuperSecret123!" | ||
| _test_expect "URL with ?access_token= param" 2 "https://api.example.com/data?access_token=eyJhbGciOiJIUzI1NiJ9" | ||
| _test_expect "URL with ?client_secret= param" 2 "https://oauth.example.com/token?client_secret=cs_abcdef123456789" | ||
| _test_expect "Short param value (no match)" 0 "https://example.com/page?key=abc" |
There was a problem hiding this comment.
Test intent mismatch: this case does not validate the “short value” threshold.
Line 1502 uses key=abc, but key is not one of the detected parameters, so this passes even if length logic regresses. Use a tracked parameter with a short value.
Suggested patch
- _test_expect "Short param value (no match)" 0 "https://example.com/page?key=abc"
+ _test_expect "Short param value (no match)" 0 "https://example.com/page?secret=abc"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _test_expect "Short param value (no match)" 0 "https://example.com/page?key=abc" | |
| _test_expect "Short param value (no match)" 0 "https://example.com/page?secret=abc" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/prompt-guard-helper.sh at line 1502, The test call to
_test_expect ("Short param value (no match)") uses "key=abc" which is not a
tracked/detected parameter so the length check isn't exercised; update the URL
argument passed to _test_expect to use one of the script's tracked parameter
names (the same param name used in other tests in this file) and give it a short
value under the threshold (e.g., param=ab) so the short-value branch in the
parameter length validation is actually tested.
…es and fix short-value threshold test - Lines 1497-1501: prefix test literals with FAKE_SK_LIVE_, FAKE_AKIA_, FAKE_JWT_, FAKE_CS_ to make it unambiguous these are test fixtures, not real credentials (CodeRabbit CHANGES_REQUESTED) - Line 1502: change 'key=abc' to 'secret=abc' — 'key' is not a tracked parameter so the test never validated the short-value threshold; 'secret' is tracked (line 330) and 'abc' (3 chars) is below the 8-char minimum, correctly producing no match Closes #4954
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Mar 15 20:48:05 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
- Add `key` to credential_exposure detection pattern (inline + YAML) - Add `key` to URL credential sanitization regex - Replace credential-like test literals with PLACEHOLDER_ prefixed values to avoid Gitleaks/secret-scanner false positives - Add test for new key= parameter detection - Short param test already used tracked param (secret=abc), confirmed correct Closes #4959
#4967) - Add `key` to credential_exposure detection pattern (inline + YAML) - Add `key` to URL credential sanitization regex - Replace credential-like test literals with PLACEHOLDER_ prefixed values to avoid Gitleaks/secret-scanner false positives - Add test for new key= parameter detection - Short param test already used tracked param (secret=abc), confirmed correct Closes #4959
2 participants
Summary
prompts/build.txt— column-level query discipline for application config tables (webhook settings, integration records, OAuth configs) that may contain embedded credentials in URL query parameterscredential_exposurepattern category detecting secrets in URL query params (?secret=,?token=,?api_key=,?password=,?access_token=,?auth=,?client_secret=,?webhook_secret=)[REDACTED]Approach
Option C from the issue — layered defense:
Testing
Files Changed
.agents/prompts/build.txt.agents/scripts/prompt-guard-helper.shcredential_exposurepatterns, URL redaction in sanitize, 9 tests.agents/configs/prompt-injection-patterns.yamlcredential_exposurecategory with 8 URL param patternsCloses #4954
Summary by CodeRabbit
New Features
Documentation
Tests