Skip to content

fix: detect masked short secrets in contains_masked_key#398

Open
developer603 wants to merge 1 commit into
dograh-hq:mainfrom
developer603:fix/mask-detection-short-secrets
Open

fix: detect masked short secrets in contains_masked_key#398
developer603 wants to merge 1 commit into
dograh-hq:mainfrom
developer603:fix/mask-detection-short-secrets

Conversation

@developer603

Copy link
Copy Markdown
Contributor

fix: detect masked short secrets in contains_masked_key

Problem

contains_masked_key() guards against persisting a still-masked secret by testing
for the MASK_MARKER ("***") substring:

return any(MASK_MARKER in k for k in keys)

But mask_key() only produces 3+ consecutive asterisks for keys longer than
VISIBLE_CHARS + 2. For short secrets it produces fewer:

mask_key("EMPTY")  # -> "*MPTY"   (a single asterisk)
mask_key("X")      # -> "*"

These masked values slip past the guard, so a dashboard "save without editing"
round-trips the masked display string back to the server and overwrites the real
stored value with e.g. "*MPTY" — silently corrupting the secret.

This is most visible for self-hosted / OpenAI-compatible providers that use a short
no-validate sentinel api_key such as "EMPTY" (common for local vLLM, Ollama, etc.).

Fix

Match the full shape mask_key() actually produces — a run of MASK_CHAR followed
by at most VISIBLE_CHARS revealed trailing characters — in addition to the legacy
marker:

_MASK_SHAPE_RE = re.compile(
    rf"^{re.escape(MASK_CHAR)}+[^{re.escape(MASK_CHAR)}]{{0,{VISIBLE_CHARS}}}$"
)
...
return any(MASK_MARKER in k or bool(_MASK_SHAPE_RE.match(k)) for k in keys)

Testing

Verified against mask_key() output and against real unmasked values:

input (real) mask_key() old detector new detector
EMPTY *MPTY ❌ False ✅ True
X * ❌ False ✅ True
mykey *ykey ❌ False ✅ True
sk-1234567890abcdef ***************cdef ✅ True ✅ True
abcd **** ✅ True ✅ True

Real unmasked values are not flagged — no false positives:
contains_masked_key("EMPTY") / ("sk-live-abcd") / ("mykey") / ("X") all return False
(no leading-asterisk run, so the anchored regex doesn't match).

python -m py_compile clean.

contains_masked_key() guards against persisting a still-masked secret by
checking for the MASK_MARKER ("***") substring. But mask_key() only emits
3+ consecutive asterisks for keys longer than VISIBLE_CHARS + 2. For short
secrets it emits fewer: e.g. mask_key("EMPTY") == "*MPTY" (a single
asterisk). Such masked values slip past the guard, so a dashboard
"save without editing" round-trips the masked display string back and
overwrites the real stored value with e.g. "*MPTY".

This bites self-hosted/OpenAI-compatible providers that use a short
no-validate sentinel api_key such as "EMPTY".

Match the full shape mask_key() produces — a run of MASK_CHAR followed by
at most VISIBLE_CHARS revealed characters — in addition to the legacy
marker. Verified: masked short secrets ("*MPTY", "*", "*ykey") are now
detected while real unmasked values ("EMPTY", "sk-live-abcd", ...) are
not, so there are no false positives.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
stefandsl added a commit to stefandsl/dograh that referenced this pull request Jun 2, 2026
…d_key

Upstream dograh-hq#398. Pure-logic fix; 38 masking/config unit tests pass.
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