Repository navigation
docs(security): document the braces/chokidar DoS finding as an accepted risk - #85
Merged
Merged
Conversation
GHSA-vfj7-8cjw-p6xm, surfaced by PR #81's Dependency Audit re-run after rebasing against main. New since Hillary's review on that PR, not the stale pre-#80 failure he was characterizing -- confirmed via npm audit directly: braces' latest release (3.0.3) is still in the vulnerable range, so there's no upgrade path yet. The only `npm audit fix --force` suggestion is downgrading @clarigen/cli (a devDependency, the Clarity codegen tool) to 0.2.4 -- a multi-major regression to dodge a DoS this project's own usage can't trigger (chokidar only ever watches our own contracts/ tree, never attacker-supplied globs). Documented inline rather than silently left red, so the next person who sees this check fail doesn't have to re-derive the same investigation -- and so it gets re-checked once a real patch exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
unixwhisperer
approved these changes
Oct 4, 2026
unixwhisperer
left a comment
unixwhisperer
left a comment
Collaborator
There was a problem hiding this comment.
Verified independently, not just read the summary:
- Root
npm audit --audit-level=highreproduces exactly: GHSA-vfj7-8cjw-p6xm (braces), path is @clarigen/cli@4.1.7 -> chokidar@3.6.0 -> braces@3.0.3. @clarigen/cliis a devDependency (package.json), used only vianpm run gen(clarigen generate) — confirmed it's not independencies.- braces 3.0.3 is npm's latest published version (
npm view braces versions) — confirmed no upstream fix exists yet. npm audit fix --forcehere does suggest@clarigen/cli@0.2.4against current^4.0.1— that's the multi-major regression described.
Reasoning holds: chokidar's only use here is watching this repo's own contracts/ tree for codegen, not attacker-controlled input, so the DoS vector isn't reachable through our own usage. Good call documenting it inline where the red check actually gets looked at instead of leaving unexplained noise. Approving.
4 tasks done
3 tasks done
unixwhisperer
pushed a commit
to unixwhisperer/Flashstack
that referenced
this pull request
Oct 12, 2026
…'s proposal Replaces the bare `npm audit --audit-level=high` at both call sites with scripts/audit-allowlist.mjs. The check has been permanently red since mattglory#85 (the accepted braces finding, GHSA-vfj7-8cjw-p6xm), and that cost real findings visibility: two separate new advisories (the source-map-js/ PostCSS trio, then sharp) went unnoticed for roughly a day because a check that's always red stops getting read. The script fails only on a finding at or above --audit-level whose advisory ID isn't already a documented, accepted-risk row in FINDINGS_REGISTER.md -- checked against that file directly, not a second list that can drift from it, the same discipline KNOWN_TEST_PATHS already applies in mainnet-plan-guard.test.ts. Matched by advisory ID, not package name, so a new advisory on an already-allowlisted package still fails. Tested against this repo's real npm audit output in both trees before wiring in: passes today, fails if an accepted ID is removed from the allowlist, and refuses to run if an allowlisted ID isn't documented in FINDINGS_REGISTER.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This branch was successfully deployed
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
2 participants
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
PR #81's Dependency Audit re-run (after rebasing to pick up #80's Next.js fix) surfaced a new, different failure -- not the stale one Hillary's review called out. Confirmed independently:
GHSA-vfj7-8cjw-p6xm(braces, stack-exhaustion DoS via deeply nested patterns), pulled in via@clarigen/cli -> chokidar -> braces.npm audit fix --force's only suggestion is downgrading@clarigen/cli(devDependency, our Clarity-to-TS codegen tool) to0.2.4-- years old, a real regression, to dodge a DoS this project's own usage can't trigger (chokidar only ever watches our owncontracts/tree here, never attacker-supplied input).Documents the finding and the reasoning inline in
security.yml, next to where it'll actually be seen, rather than leaving it as an unexplained red check someone has to re-investigate from scratch. Says explicitly to re-check once a real patch lands.Test plan
npm audit --audit-level=highnext time this step goes red, per the comment, to confirm whether this is still the same finding.🤖 Generated with Claude Code