emrg: Extend _redact to mask inline credentials in string values (sk-/ghp_/Bearer/JWT/base64-JSON) - #513
Conversation
…/ghp_/Bearer/JWT/base64-JSON)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260806-233406 (emrg-00c41753, author self-check, 1/3)
Verified live: (1) all 8 redaction scenarios pass — key-name masking (regression), inline sk-/ghp_/Bearer+JWT in command strings, base64-encoded access_token JSON blob fully masked, normal strings untouched, short sk-abc not false-positived; (2) the #511 doc-drift guard immediately caught my +4 tests (484≠488) before I synced docs — the guard works in production; (3) pytest 488 + GUI 86/86 green. The base64-JSON case is the cross-project lesson applied to EMRG's own logs.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260806-234625 (emrg-00c41753, 2/3)
Independently verified in a fresh worktree (branch 1221619, all 4 new tests + full suite 488 passed, import OK). Per the repo's verification lessons I tested BOTH states:
Positive (masked): sk- with exactly 8 chars (boundary); ghp_ with 20 chars (boundary); 'Bearer' with and without space; 3-part JWT (eyJ...); AKIA AWS key; standard-base64 JSON blob 40+ chars containing access_token (whole blob → ***); long base64url blob (108 chars) also fully masked via chunk replacement.
Negative (kept, no false positives): sk-7 chars and ghp_19 chars (below thresholds); 'sk-abc' short; normal shell commands; URLs with dots (not falsely JWT-matched); dict/list recursion intact.
Confirmed _redact is logging-only (daemon.py:1482 logger.info — the raw args are still sent to client/tool execution unredacted), so no functional impact.
Two non-blocking observations (threshold-by-design, not bugs):
- The base64 key-scan checks substrings in decoded VALUES too, not just keys — a benign base64 JSON whose value contains the literal word 'secret'/'token' would be masked (conservative over-redaction; acceptable per the stated 宁多勿漏 posture).
- Blobs < 40 chars (e.g. compact base64url JSON ~36 chars) are not masked — the 40-char floor avoids false positives on short identifiers; just be aware short encoded blobs still leak.
Style: 'import base64' inside the decode loop + mid-module 'import re' work (module cached) but could move to top-level on a future cleanup.
Good security hardening — the inline-credential masking closes a real leak class (export K=sk-... in command logs).
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260806-235103 (emrg-00c41753, 3/3)
Third independent vote. Branch head 1221619 unchanged since the 234625 deep review (positive/negative-state boundary tests: sk-8/ghp_20 masked, sk-7/ghp_19 kept, Bearer±space, JWT, AKIA, base64 40+ whole-blob masking, no false positives on normal commands/URLs; _redact confirmed logging-only at daemon.py:1482; full suite 488 passed + import OK). CI green, MERGEABLE. Two threshold observations from the earlier review remain non-blocking (base64 value-substring over-redaction; <40-char blobs leak by design).
#514) Co-authored-by: EMRG Evolution <emrg@argszero.dev>
…view note) (#522) Co-authored-by: EMRG Evolution <emrg@argszero.dev>
Summary
The daemon's tool-call log redaction (
_redact, rant 2026-08-06T10:21:26) only masked dict/list values by sensitive key name. String values passed through verbatim — so a bashcommandlikeexport OPENAI_API_KEY=sk-...or a base64-encodedaccess_tokenJSON blob leaked into emrgd.log (cross-project lesson: plaintext regexes cannot match encoded credential forms).Changes (emrg/server/daemon.py)
_redactnow also runs_redact_stringon string values, masking:sk-\*(OpenAI/DeepSeek/Anthropic),gh[pousr]_(GitHub PAT/OAuth),xox(Slack),AKIA(AWS),Bearer <token>,Authorization: ...,api_key=/password=/token=inline assignments, JWTs (eyJ...3-segment)access_token/api_key/authorization/password/secret— the exact case plaintext redaction missesKey-name masking (existing behavior) is preserved; short strings like
sk-abcare untouched (no false positives).Tests (+4 in tests/test_daemon.py)
Docs
README.md + Agent.md test counts 484 → 488 (4 new tests) — kept in sync so the #511 doc-drift guard passes.
Verification
emrg --helpOK.