chore: P0/P3 credibility and robustness cleanup (6 issues) - #60
Merged
Conversation
src/redactor/masker.ts was never imported anywhere in src/ or scripts/ (verified via grep). Redaction is actually handled by PrivacyScanner.scan()/buildHookResponse() in engine.ts and response.ts. Dead code in a security-audited repo invites doubt about what's load-bearing. Closes #49
.claude-plugin/marketplace.json was stuck at 0.1.2 while package.json/plugin.json were at 0.2.8 - eight releases of drift, because scripts/release.sh only ever bumped plugin.json. Also gave the marketplace listing's description a trailing period to match normal sentence punctuation. - Bump marketplace.json to the current version (0.2.8) - Have release.sh bump marketplace.json's plugin version alongside package.json/plugin.json so this can't drift again Closes #47
… run
Three entries in data/regex_list_1.json survive the coding-only
keyword filter but were rejected by safe-regex2 on every single hook
invocation, printing a warning and wasting load work while
contributing zero detections:
- "Password etc shadow": the /etc/shadow hash branch nested an
optional group around unbounded .* repetitions (star height 2).
Rewrote it as a non-optional, delimiter-bounded [^$:]* match - same
intent (locked-account markers or a $id$salt$hash), no nesting.
- "heroku_key": (?:-\w{4}){3} nested a bounded repetition around
another repetition. Unrolled it to -\w{4}-\w{4}-\w{4} - identical
match, no nesting.
- "slack_api_token": (?:-[a-zA-Z0-9]+){4,} nests two unbounded
repetitions, a genuine catastrophic-backtracking shape. Removed
outright rather than reworked - it's a strict subset of the
existing built-in 'slack-token' detector, which already covers
xoxb/xoxp/xoxa/xoxr/xapp with better false-positive guards.
Closes #48
ConfigLoader only ever validated the "mode" field. Any other typo'd key (e.g. "alowedValues" instead of "allowedValues") was silently merged over the defaults and just as silently ignored - for a security tool, a config typo that silently weakens protection with no feedback is a real footgun. ConfigLoader now checks every top-level key against the known PrivacyGuardConfig fields and prints a console.warn naming the offending key. Unknown keys are still merged through unchanged (not stripped), preserving the config-UI's "unknown keys survive a save" behavior. Closes #50
Setting "enabled": false in .privacy-guard.json silently let every prompt through unscanned with no in-session signal - a stale config line left over from debugging meant a user could be completely unprotected without any indication. The hook now emits a systemMessage the first time it sees a given session_id with the guard disabled, then stays quiet for the rest of that session (tracked via a small marker file in the existing cache dir, keyed on session_id) so it isn't repeated on every prompt. Falls back to notifying every time only when no session_id is available (e.g. the script invoked manually without the JSON envelope), since there's nothing to dedupe against. Closes #51
Windows support is hand-coded (path handling, %LOCALAPPDATA% config resolution, no bash wrapper by design) and macOS is the primary dev platform for Claude Code users, but CI only ever ran ubuntu-latest. Untested platform claims rot silently. - Keep the full Node 18/20/22 matrix on ubuntu-latest, add one windows-latest and one macos-latest job on Node 20 via `include` (fail-fast: false so one platform's failure doesn't hide another's) - Force LF line endings repo-wide via .gitattributes (`* text=auto eol=lf`) - without this, Windows checkouts convert committed LF to CRLF via core.autocrlf, which would make the "dist/ matches src/" verification step fail spuriously on windows-latest even when nothing actually changed Closes #52
Two bugs surfaced by the new windows-latest job (#52): 1. The "Verify committed dist/ matches src/" step uses bash `if ! cmd; then` syntax, but Windows runners default `run:` steps to PowerShell - which fails to parse it outright (ParserError: Missing '(' after 'if' in if statement), unrelated to whether dist/ actually matched. Set `defaults.run.shell: bash` on the job so every step runs under bash on all three platforms. 2. Once that parses, the check would still have failed spuriously on Windows: tsc's `newLine` option defaults to the platform's native line ending, so a Windows build emits CRLF while the committed dist/ (LF) would not match even with nothing actually changed. Set `"newLine": "lf"` in tsconfig.json so tsc always emits LF regardless of OS, independent of the .gitattributes normalization added in the prior commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Six small, independent fixes from the product audit's "lowest hanging fruit" bucket - each closes one open issue. Every commit is self-contained and can be reviewed/reverted individually.
src/redactor/masker.ts(unusedRedactorclass).claude-plugin/marketplace.jsonversion (was stuck at 0.1.2 vs actual 0.2.8);release.shnow bumps it automaticallyslack-tokenrule and was removed instead of reworkedConfigLoadernow warns on unrecognized config keys (e.g. a typo likealowedValues) instead of silently ignoring themenabled: falsenow emits a one-time-per-sessionsystemMessageinstead of silently disabling the guard with zero in-session signalwindows-latestandmacos-latest(Node 20) alongside the existing ubuntu 18/20/22 matrix; also fixes a latent.gitattributesgap that would've made the dist-verification step fail spuriously on Windows due to CRLF conversionTest plan
npm run build- cleannpm test- 377/377 passing (2 new test files: config unknown-key warnings, disabled-notice behavior)npm run lint- clean