Skip to content

fix(secrets): mask underscore-adjacent credential key names - #647

Merged
plind-junior merged 4 commits into
vouchdev:testfrom
galuis116:fix/mask-secrets-underscore-boundary
Jul 30, 2026
Merged

fix(secrets): mask underscore-adjacent credential key names#647
plind-junior merged 4 commits into
vouchdev:testfrom
galuis116:fix/mask-secrets-underscore-boundary

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

What changed

_ASSIGNMENT in src/vouch/secrets.py now delimits its credential-keyword
alternation with alphanumeric lookaround ((?<![A-Za-z0-9])...(?![A-Za-z0-9]))
instead of \b...\b, so it matches keywords inside underscore-delimited
names like access_token, client_secret, DB_PASSWORD, and
AWS_SECRET_ACCESS_KEY.

Why

\b treats _ as a word character, so \btoken\b never matches token
inside access_token — the _t transition is \w\w, never a
boundary. Same for secret inside client_secret and password inside
DB_PASSWORD. Since snake_case / SCREAMING_SNAKE_CASE is the dominant
real-world shape for credential env-vars (.env files, shell export,
docker-compose), this excluded the majority of actual credentials from
masking — only a bare token=/secret=/password= with no prefix or
suffix was ever caught.

This regex backs two live call sites: the capture-time guard (a secret
that reaches the buffer is permanent, per the module's own docstring) and
lifecycle.redact(), the manual remediation backstop for a credential
that already reached a durable claim. Both were silently missing the
common case.

Fixes #646

What might break

Nothing for users with an existing .vouch/ directory — no on-disk shape,
kb.* method, or object model change. Behaviorally, mask_secrets() now
redacts more inputs than before (strictly a superset): any text that
previously matched a bare keyword (token=, password=, etc.) still
matches identically; text with a snake_case-prefixed/suffixed keyword
(access_token=, DB_PASSWORD=) now also matches, where it previously
passed through untouched. No previously-masked case becomes unmasked.

VEP

Not applicable — no object model, kb.* method, on-disk layout, bundle
format, or audit-log shape change. A regex-correctness fix inside a single
pure string function.

Tests

  • make check-equivalent locally: ruff clean; mypy clean on
    secrets.py; all 19 tests/test_secrets.py cases pass (17
    pre-existing + 2 new). Also ran the directly-affected
    test_capture.py, test_capture_answer.py, test_capture_scope.py,
    test_cli_lifecycle_surface.py, and test_lifecycle.py suites —
    one pre-existing failure (test_finalize_supersedes_updated_claims)
    confirmed present identically on test HEAD before this change
    (via git stash comparison), unrelated to secrets.py
  • New / changed behaviour has a test —
    test_masks_underscore_adjacent_key_names,
    test_underscore_boundary_change_has_no_new_false_positive
  • CHANGELOG.md updated under ## [Unreleased]

_ASSIGNMENT wrapped its keyword alternation in \b...\b, but \b treats
_ as a word character, so \btoken\b never matched token inside
access_token, and likewise for secret inside client_secret /
AWS_SECRET_ACCESS_KEY and password inside DB_PASSWORD. snake_case and
SCREAMING_SNAKE_CASE are the dominant real-world shape for these
env-vars (.env files, shell export, docker-compose), so this excluded
most actual credentials from masking — only a bare token=/secret=/
password= with no prefix or suffix was ever caught.

replace the \b boundaries with explicit alphanumeric lookaround so
underscore-delimited segments match while true false positives like
tokenized= or passwordless= stay excluded. this regex backs both the
capture-time guard (a secret that reaches the buffer is permanent) and
lifecycle.redact(), the manual remediation backstop for a credential
that already reached a durable claim — both were silently missing the
common case.

Fixes vouchdev#646
@github-actions github-actions Bot added docs documentation, specs, examples, and repo guidance tests tests and fixtures size: XS less than 50 changed non-doc lines labels Jul 30, 2026
two prs independently added a test of the same name asserting the same
invariant (archived followup pages excluded from digest) with different
fixtures. python silently shadows the first with the second, and ruff's
F811 flags it as dead code — this fails ruff check src tests for every
pr into test right now, unrelated to what that pr actually changes.

rename the first (relative-dates, multi-page fixture) instead of
deleting it, so both scenarios keep running.
@galuis116

Copy link
Copy Markdown
Contributor Author

added a second, unrelated commit: ruff check src tests was red for every open PR against test right now (a duplicate test_build_excludes_archived_followups name in tests/test_digest.py from two independently-merged fixes, F811). renamed the shadowed one instead of deleting it, so both scenarios still run. flagging in case it is worth a heads-up to other in-flight PRs too.

@plind-junior
plind-junior merged commit 2a472f4 into vouchdev:test Jul 30, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs documentation, specs, examples, and repo guidance size: XS less than 50 changed non-doc lines tests tests and fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants