ci: let the dependency audit fail

[mattglory]

Summary

Removes continue-on-error: true from both npm audit --audit-level=high steps in security.yml, so the Dependency Audit job can actually go red.

Why

With the flag set, the job passed unconditionally. Evidence from PR #61's Security Scan run (before the deps fix in #63): the audit step printed several Severity: high findings and the job's conclusion was success. It stayed green while GitHub showed one critical and six high alerts. A check that can't fail isn't evidence of anything, which is the F-7 shape, and it's what a Slack "Security CI" summary (proposed by Hillary) would have reported as all-clear. This should land before any such notification.

Safe to merge now

• Both audits exit 0 today (root: 0 vulnerabilities after chore(deps): clear the critical and all high Dependabot alerts #63; web: 0 high/critical), so this doesn't turn the check red now, only when a new high/critical advisory lands.
• Dependency Audit is not a required check (Build Frontend and Test Smart Contracts are), so a red run is a signal and never blocks a merge.
• The workflow still parses (validated), and only that file changed.

Trade-off worth knowing

A new advisory can turn Dependency Audit red on an unrelated PR. That's the intended signal, but it can look like noise. The weekly Monday run means it's caught even without PR traffic. npm audit also depends on the registry being reachable, so a transient network failure will show as red too.

.github/workflows/ is CODEOWNERS-protected (Hillary).

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
web	Ready	Preview	Sep 21, 2026 12:07pm UTC

[unixwhisperer]

Approving — the change is right and I'm the workflow code owner, so this was waiting on me. One thing you need to know before you hit merge, because it contradicts the PR body.

The "safe to merge now" premise no longer holds

The PR says:

Both audits exit 0 today (root: 0 vulnerabilities after #63; web: 0 high/critical), so this doesn't turn the check red now, only when a new high/critical advisory lands.

That is no longer true. I reproduced CI exactly — clean git worktree checkout of origin/main, no npm ci (the workflow doesn't run one), npm audit --audit-level=high in both locations:

root:  exit 1   {critical: 1, high: 8, moderate: 4, total: 13}
web:   exit 1   {critical: 0, high: 2, moderate: 16, low: 5, total: 23}

Lockfiles verified byte-identical to main, so this is the same dependency set CI audits. Merging this will turn Dependency Audit red on the next run, immediately — not "only when a new advisory lands."

What's flagged — all dev/test tooling, no contract surface

critical  vitest          Vitest UI server: arbitrary file read + execute (via vite)
high      vite            path traversal in optimized-deps .map handling
high      ws              DoS via many HTTP headers; uninitialized memory disclosure
high      smol-toml       DoS via malformed TOML
high      @clarigen/*     transitive, via @stacks/blockchain-api-client -> socket.io-client, ws

Worth saying plainly: none of this touches the contracts. The contracts are Clarity; this is the JS test/build harness. The vitest/vite critical requires the Vitest UI server to be listening, which it isn't in CI. So this is real but low-urgency, and it is not a reason to hold the PR.

Why I'm still approving, and in fact more strongly

This makes your case better, not worse. Your #61 evidence was that a check could stay green while findings printed. The current state is the stronger version: there is 1 critical and 8 high in the tree right now, and Dependency Audit is green on main today. That is precisely the false-assurance shape you're removing, sitting live rather than in a past run.

Your two safety arguments both survive intact: Dependency Audit is not a required check, so red never blocks a merge; and a signal that can't go red isn't a signal. Merging this makes CI start telling the truth. The truth just happens to be red on day one.

What I'd ask

1. Expect the red and don't read it as regression. If Dependency Audit goes red the moment this lands, that's the change working. Worth a line in the merge comment so neither of us re-litigates it in a week.
2. Don't let it sit red indefinitely. A permanently-red check gets ignored exactly like a permanently-green one — different failure, same end state. Most of this resolves through the vite/vitest upgrade path; the @clarigen/* chain is transitive and may need to wait upstream. Happy to take the triage as a follow-up and file what can't be fixed as an accepted-risk row in the findings register, the way F-6 is handled.
3. Unrelated but worth knowing: I could not explain why this PR's own Security Scan run went green on 21 Sep with the flag already removed — the advisories involved predate that run (smol-toml 9 Sep, ws and vite June). Same lockfile, same command. I'd re-run the workflow on this branch before merging to see today's answer rather than the cached one. Not a blocker, but I'd rather flag an unexplained green than assume it away — that's the same instinct this PR is built on.

Verified, no action needed

• Diff is exactly the two continue-on-error: true removals plus the explanatory comment. Nothing else in the file.
• YAML still parses.
• codeql job untouched.
• .github/workflows/ CODEOWNERS approval: that's this review.

This unblocks Slack Stage 2 (Flashstack-ajv.2.9) — once audit failures are real workflow failures, a Slack security summary can report something meaningful. Stage 1 spec is in #72; Stage 2 stays unbuilt until this is on main.

[unixwhisperer]

Correction: my audit numbers in the review above are wrong. Disregard that section. Matt reproduced and couldn't get them, and he's right — I measured a stale tree.

Root cause

My verification worktree was cut from origin/main, but my origin/main ref was 19 hours stale. The reflog is unambiguous:

00cf0c4  origin/main@{2026-09-22 16:24:36}: fetch origin -q: fast-forward
737c659  origin/main@{2026-09-21 10:14:09}: fetch origin --prune: fast-forward

I cut the worktree at ~15:43 on the 22nd, so it came from 737c659. My git fetch didn't happen until 16:24 — after the measurement, during branch cleanup.

737c659 predates #63 (e07c01e, "clear the critical and all high Dependabot alerts"). The lockfile moved 823 insertions / 1250 deletions between that ref and current main. So I audited the pre-#63 baseline and reported it as current — exactly Matt's inference, including that the numbers match pre-#63 digit for digit.

My "lockfiles verified byte-identical to main" check didn't catch it because it compared stale-to-stale — my working branch was also cut from 737c659. Two stale things agreeing looked like corroboration. That check was worthless in precisely the case it was meant to guard.

Correct numbers, re-measured on 00cf0c4

root:  exit 0   {critical: 0, high: 0, moderate: 0, total: 0}
web:   exit 0   {critical: 0, high: 0, moderate: 15, low: 5, total: 20}

Matching Matt's independent run and the actual Actions log. The PR body's "both audits exit 0 today" was correct as written. There was no critical, no high, and nothing for the check to catch — the green CI run was never mysterious, because #69 branched off main after #63 merged.

What this retracts

• The claim that merging turns Dependency Audit red immediately — false, it stays green.
• The "1 critical + 8 high exist right now while the check is green" framing — false, and it was the one part of my review arguing the change was more urgent than stated. It wasn't.
• My related claim elsewhere that Dependabot PR Bump smol-toml from 1.7.0 to 1.8.0 #41 was an unmerged fix for a live high advisory — also false. smol-toml is already 1.8.0 on main; chore(deps): clear the critical and all high Dependabot alerts #63 handled it, so Bump smol-toml from 1.7.0 to 1.8.0 #41 is superseded, not neglected.
• The follow-up triage issue I filed for those advisories is closed as invalid.

What stands

The approval, and the reasoning for it that doesn't depend on the numbers: a check that can't go red isn't evidence, Dependency Audit isn't a required check so red never blocks a merge, and the #61 run is real historical evidence of the false-green. Those were the arguments in your PR body and they carried it on their own.

The process lesson is mine to take: I ran the right verification and drew a confident conclusion from an unfetched ref, then treated a stale-vs-stale comparison as confirmation. Fetching first is now the first step of any measurement I report, and a same-ref comparison isn't corroboration.

[mattglory]

Appreciate the trace-down — that's exactly the right way to retract something. No action needed on my end; #69 already merged with the correct framing, and I've closed #41 and #46 as redundant.