redact: catch high-entropy SESSION keys (BW_SESSION) and slashed base64 values - #4
Conversation
envctl redact missed Bitwarden's BW_SESSION: SESSION only counted as a secret signal when paired with KEY (SESSION_KEY), so suspicious_key_name never let the high-entropy base64 session key reach the entropy detector. Add SESSION to the suspicious segment list. The entropy bar still keeps benign values (SESSION_TIMEOUT=3600, SESSION_NAME=my-app-session) and the *_ID identifier exemption (SESSION_ID) unmasked.
Standard base64 includes '/', so roughly 40% of real 64-byte session keys (Bitwarden BW_SESSION and similar) carry two or more slashes and were rejected by entropy_secret's relative-path guard before their entropy was measured. Trailing '=' padding on a base64-charset value is decisive evidence of encoded data over a path, so accept slashed values only in that shape. Path-like values keep the guard: they neither end in '=' nor survive the entropy bar.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2026-07-31T02:14:35.239ZApplied to files:
🔍 Remote MCP DeepWiki, GitHub GrepAdditional review context
🔇 Additional comments (2)
📝 WalkthroughUpdate
156 tests pass. One FIFO timing test fails on the unchanged base commit. WalkthroughEntropy-based redaction now recognises keys containing Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (5 passed)
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: 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 `@src/redact.c`:
- Around line 430-446: Replace the weak b64/padded slash-guard exception in the
token classification loop with validation that confirms standard Base64
alphabet, valid trailing padding position and count, and length alignment before
bypassing the relative-path rejection. Keep slash-containing values rejected
unless they pass this complete Base64 validation, and add a regression case
covering a path-like high-entropy value with multiple slashes ending in “=”.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 834c3862-323c-474f-8607-a64db5b77584
📒 Files selected for processing (3)
README.mdsrc/redact.ctests/cases/filter-session-keys.case
📜 Review details
🧰 Additional context used
🔍 Remote MCP DeepWiki, GitHub Grep
Relevant review context
- Bitwarden’s CLI generates
BW_SESSIONas a random 64-byte symmetric key, base64-encodes it, exposes it through the environment or--session, and later decodes it for encryption/decryption. This confirms it is a sensitive credential rather than a benign session label. - Official Bitwarden code sets
process.env.BW_SESSION = key.toBase64()and documents exporting it afterbw unlock; the key is also cleared on lock. - Real integrations consistently use
export BW_SESSION=$(bw unlock --raw)and pass it tobwcommands, includingjdx/fnoxdocumentation/tests and several shell integrations. jdx/fnoxtests explicitly requireBW_SESSIONfor authenticated Bitwarden operations and validate it withbw status --session "$BW_SESSION", supporting regression coverage for environment-variable redaction.- The target
kjanat/envctlrepository was not indexed by DeepWiki, and repository-scoped GitHub Grep queries returned no results; validation of the changed implementation itself was therefore limited to the supplied PR context.
🔇 Additional comments (3)
src/redact.c (1)
117-117: LGTM!README.md (1)
192-193: LGTM!tests/cases/filter-session-keys.case (1)
1-21: LGTM!
The bypass accepted any authz-charset value whose last byte was '=', so '.', '_', and '-' could appear in the "base64" body and only the final byte was inspected. Require the real thing before trusting the padding signal: '+/' alphabet only, 4-byte alignment, and at most two '=' confined to the tail. A path-shaped value like lib/session_v2/loader.so= now stays on the path side of the guard, pinned by a new regression line.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as low quality.
This comment was marked as low quality.
4b8434d to
f3aef4d
Compare
The OAUTH, BEARER, JWT, and CRED segments routed values to the entropy detector with no test asserting that a high-entropy value under them actually masks; only the OAUTH_CLIENT_ID exemption side was pinned. Cover the masking side for all four, plus the *_ID exemption and a low-entropy guard, so the suspicious-segment list cannot quietly regress.
f3aef4d to
45d7020
Compare
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
Pin filter-mode behavior for every detector family, in seven case files totaling ~173 verified input behaviors: - filter-entropy-key-segments: all ten suspicious segments masking high-entropy values, with a guard per segment (paths, UUIDs, trivial values, *_ID identifiers). - filter-strong-keys: every strong key name masking on sight, plus trivial-value, pathish-suffix, and *_ID exemptions. Pins that a bare PASSWORD=changeme line masks as a connection-string fragment before the trivial exemption is reached. - filter-token-prefixes: all known token prefixes and the AWS A3T shape, JWT compact form, bare-prefix and too-short guards. - filter-value-shapes: credentialed URLs, sensitive URL params, Authorization values, connection strings, private JWK, inline PEM, and the public-PEM, path, digest, and UUID non-masking guards. - filter-json-yaml-contexts: JSON pairs, YAML colon keys, quoted export lines, nested JSON, private JWK, and JSON-side exemptions. - filter-prose-lines: secrets embedded in log and prose lines, header values, Set-Cookie, and prose digest/UUID/commit guards. - filter-exemption-boundaries: entropy class length floors (hex 32, base64 24, opaque 16 requiring a non-base64 char), low-entropy and short-value guards, pathish and digest suffixes, trivial values, and the VAULT_SECRET_ID strong-over-identifier override. All expectations verified against the binary line by line before pinning; suite grows from 157 to 163 cases.
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 `@tests/cases/filter-strong-keys.case`:
- Around line 32-34: Update mask_tokens so ordinary KEY=VALUE tokens do not fall
through to conn_string_secret after the placeholder exemption and get
reclassified as secrets. Preserve PASSWORD=changeme unchanged in the
filter-strong-keys fixture while retaining masking for genuine connection-string
secrets.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3cb18540-4406-4c13-89f8-886497f19ad3
📒 Files selected for processing (7)
tests/cases/filter-entropy-key-segments.casetests/cases/filter-exemption-boundaries.casetests/cases/filter-json-yaml-contexts.casetests/cases/filter-prose-lines.casetests/cases/filter-strong-keys.casetests/cases/filter-token-prefixes.casetests/cases/filter-value-shapes.case
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: windows-2025-vs2026
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (c-cpp)
🧰 Additional context used
🪛 Betterleaks (1.7.0)
tests/cases/filter-prose-lines.case
[high] 17-17: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 12-12: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tests/cases/filter-token-prefixes.case
[high] 21-21: Discovered a DigitalOcean Personal Access Token, posing a threat to cloud infrastructure security and data privacy.
(digitalocean-pat)
[high] 27-27: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
[high] 30-30: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 18-18: Identified a Slack Bot token, which may compromise bot integrations and communication channel security.
(slack-bot-token)
[high] 16-16: Found a Stripe Access Token, posing a risk to payment processing services and sensitive financial data.
(stripe-access-token)
[high] 17-17: Found a Stripe Access Token, posing a risk to payment processing services and sensitive financial data.
(stripe-access-token)
[high] 7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tests/cases/filter-exemption-boundaries.case
[high] 6-6: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 8-8: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 12-12: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 37-37: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tests/cases/filter-strong-keys.case
[high] 22-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tests/cases/filter-entropy-key-segments.case
[high] 6-6: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 8-8: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tests/cases/filter-json-yaml-contexts.case
[high] 6-6: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 8-8: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 16-16: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 19-19: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
tests/cases/filter-value-shapes.case
[high] 8-8: Detected a MongoDB connection string with embedded credentials, potentially exposing direct database access and sensitive application data.
(mongodb-connection-string)
[high] 10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 12-12: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 19-19: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 GitHub Check: GitGuardian Security Checks
tests/cases/filter-prose-lines.case
[error] 10-10: GitGuardian detected a hardcoded Basic Auth String. Revoke/rotate it and replace it with a safe test placeholder.
[error] 7-7: GitGuardian detected a hardcoded Bearer Token. Revoke/rotate it and replace it with a safe test placeholder.
[error] 12-12: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
[error] 17-17: GitGuardian detected a hardcoded JSON Web Token. Revoke/rotate it and replace it with a safe test placeholder.
[error] 13-13: GitGuardian detected a hardcoded X-API-Key Secret. Revoke/rotate it and replace it with a safe test placeholder.
tests/cases/filter-token-prefixes.case
[error] 7-7: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
[error] 16-16: GitGuardian detected a hardcoded Stripe Key. Revoke/rotate it and replace it with a safe test placeholder.
[error] 27-27: GitGuardian detected a hardcoded Google API Key. Revoke/rotate it and replace it with a safe test placeholder.
[error] 21-21: GitGuardian detected a hardcoded DigitalOcean Personal Access Token v1. Revoke/rotate it and replace it with a safe test placeholder.
[error] 30-30: GitGuardian detected a hardcoded JSON Web Token. Revoke/rotate it and replace it with a safe test placeholder.
[error] 18-18: GitGuardian detected a hardcoded Slack Bot Token. Revoke/rotate it and replace it with a safe test placeholder.
tests/cases/filter-exemption-boundaries.case
[error] 7-7: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
tests/cases/filter-strong-keys.case
[error] 22-22: GitGuardian detected a hardcoded Generic Encryption Key. Revoke/rotate it and replace it with a safe test placeholder.
tests/cases/filter-entropy-key-segments.case
[error] 8-8: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
tests/cases/filter-json-yaml-contexts.case
[error] 8-8: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
[error] 19-19: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
[error] 7-7: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
[error] 6-6: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
[error] 10-10: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
[error] 16-16: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
[error] 21-21: GitGuardian detected a hardcoded Generic Password. Revoke/rotate it and replace it with a safe test placeholder.
[error] 9-9: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
[error] 17-17: GitGuardian detected a hardcoded Generic Password. Revoke/rotate it and replace it with a safe test placeholder.
tests/cases/filter-value-shapes.case
[error] 17-17: GitGuardian detected a hardcoded Generic Password. Revoke/rotate it and replace it with a safe test placeholder.
[error] 10-10: GitGuardian detected a hardcoded Generic High Entropy Secret. Revoke/rotate it and replace it with a safe test placeholder.
🔍 Remote MCP DeepWiki, GitHub Grep
Additional review context
- Bitwarden’s official CLI generates
BW_SESSIONfrom an AES-256-CBC-HMAC key and stores the Base64 representation in the environment. It explicitly prints shell export examples and supports passing the same value via--session, confirming thatBW_SESSIONcontains authentication material. - Bitwarden’s CLI also exposes
--session <session>as an alternative to reading the session key from the environment. - Real integrations use
export BW_SESSION=$(bw unlock --raw), pass it to authenticated commands, and sometimes persist it in shell or tmux state. This supports covering assignment-style, structured, and persisted configuration inputs. - Related code validates Bitwarden session values using a Base64-like character set including
+,/, and=with a minimum length, supporting the PR’s need to handle slash-containing padded Base64 values carefully. - No source implementation could be retrieved for
kjanat/envctl: GitHub Grep returned no matches for the target functions, and DeepWiki reports that the repository is not indexed.
🔇 Additional comments (6)
tests/cases/filter-entropy-key-segments.case (1)
6-49: LGTM!tests/cases/filter-exemption-boundaries.case (1)
6-53: LGTM!tests/cases/filter-json-yaml-contexts.case (1)
6-39: LGTM!tests/cases/filter-prose-lines.case (1)
6-31: LGTM!tests/cases/filter-token-prefixes.case (1)
6-32: LGTM!tests/cases/filter-value-shapes.case (1)
6-57: LGTM!
mask_tokens re-evaluates a declined assignment as one whole token, so PASSWORD=changeme — already exempted as a trivial placeholder by the keyed check — fell through to conn_string_secret, which matched the leading password= and swallowed the key as <redacted:credentials>. Skip the whole-token fallback when the token parses as KEY=VALUE with a trivial value. Non-trivial conn fragments (password=verysecret, sslkey=/path), compound tokens (Server=db;Password=...), and the value-level connection-string check keep masking.
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 `@src/redact.c`:
- Around line 1137-1141: Restrict the trivial_kv bypass in the assignment
parsing flow to explicit placeholder literals only, rather than all values
accepted by is_trivial_value, so numeric credential values such as
PASSWORD=123456 still reach the generic token detector and conn_string_secret
masking. Use or add a narrower placeholder helper at this condition and add
regression coverage for signed and unsigned numeric password values.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 772e9c1a-fc2b-4bd5-9216-b77ea89aa604
📒 Files selected for processing (2)
src/redact.ctests/cases/filter-strong-keys.case
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: windows-2025-vs2026
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (c-cpp)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-07-31T02:14:35.239Z
Learnt from: kjanat
Repo: kjanat/envctl PR: 2
File: tests/cases/get-env-missing.case:1-4
Timestamp: 2026-07-31T02:14:35.239Z
Learning: Test case fixtures under tests/cases are executed by tests/run.sh through env -i with a controlled PATH and only variables declared in the case's %% setenv section. Do not assume developer-shell or CI environment variables are inherited; variables omitted from %% setenv, such as NO_SUCH_VAR, are deterministically absent.
Applied to files:
tests/cases/filter-strong-keys.case
🔍 Remote MCP GitHub Grep
Additional review context
- Bitwarden CLI generates
BW_SESSIONviaSymmetricCryptoKey.toBase64()and assigns it toprocess.env.BW_SESSION, confirming the value is Base64-encoded key material. - Bitwarden explicitly prints
BW_SESSIONshell-export examples and supports the same value through--session. - The CLI decodes
BW_SESSIONwithUtils.fromB64ToArray()and constructs a symmetric key from it, confirming that/and=are valid Base64 characters that must not be rejected indiscriminately. - Bitwarden clears
BW_SESSIONwhen locking, and its CLI option handler setsBW_SESSIONfrom--session; both environment and argument-based usage are therefore relevant redaction inputs. - No source matches were available for
kjanat/envctl’s implementation or fixtures through GitHub Grep, so repository-specific validation beyond the supplied PR context was unavailable.
🔇 Additional comments (2)
src/redact.c (1)
117-117: LGTM!Also applies to: 424-439, 450-461
tests/cases/filter-strong-keys.case (1)
1-44: LGTM!Also applies to: 45-83
is_trivial_value also accepts signed and unsigned numerics up to six digits, so the token-fallback bypass introduced for PASSWORD=changeme let PASSWORD=123456 — a plausible PIN — skip conn_string_secret. Split the word list into is_trivial_word and gate the bypass on that alone; short numerics keep reaching the shape detectors. Regression lines pin PASSWORD=123456 masking as a connection-string fragment and the signed +123456 masking through the keyed path (seven characters exceeds the numeric-trivial limit).
1 participant
Fixes the missed redaction of Bitwarden's
BW_SESSIONreported viaenvctl redact < settings.json, whereTRUENAS_API_KEYin the same file was masked but the session key passed through untouched.Root cause
Two independent gaps, each fixed in its own commit:
Key vocabulary:
SESSIONonly counted as a secret signal when paired withKEY(e.g.SESSION_KEY).suspicious_key_name()— the gate deciding whether a value reaches the entropy detector at all — didn't includeSESSION, soBW_SESSIONreturned "not a secret" before its value was ever examined.Slash path-guard: even with the key fixed,
entropy_secret()rejected any value containing two or more/characters (a relative-path guard). Standard base64 includes/in its alphabet, so roughly 40% of real 64-byte session keys would still have leaked. Trailing=padding on a base64-charset value is decisive evidence of encoded data over a path, so slashed values are now accepted in exactly that shape.Behavior
BW_SESSION=<88-char base64>masks in env, assignment, and JSON forms — including values containing/.SESSION_TIMEOUT=3600,SESSION_NAME=my-app-session,SESSION_ID(existing*_IDexemption), and path values likeSESSION_KEY_PATH=config/keys/session.pem(no padding, fails the entropy bar).API_KEY_FILE=src/main/resources/key.txtremains unmasked.Testing
tests/cases/filter-session-keys.casecovering the JSON form from the report, plain assignments with and without slashes, and the four benign guard shapes.filter-epipe-reports-write-failure) is a fifo-timing case that fails identically on the unmodified base commit in this sandbox.Related: #3 tracks the remaining false-negative classes found while auditing this.