Skip to content

chore(audit): execute 2026-06-02 audit — security invariants, fail-closed guardrails, audit-trail completeness, queue+MFA tests - #115

Merged
agjs merged 17 commits into
mainfrom
chore/audit-fixes-20260602-1932
Jun 2, 2026
Merged

chore(audit): execute 2026-06-02 audit — security invariants, fail-closed guardrails, audit-trail completeness, queue+MFA tests#115
agjs merged 17 commits into
mainfrom
chore/audit-fixes-20260602-1932

Conversation

@agjs

@agjs agjs commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Executes the 2026-06-02 19:32 audit report (produced by audit-monorepo, executed via execute-audit). Every finding was independently verified against main before any fix — 5 parallel read-only review agents checked all 22 findings' evidence. 17 findings fixed, 4 rejected with evidence, 1 confirmed as documented design. 0 failed.

Verification outcome (22 findings)

  • 16 confirmed (2 with corrections), 3 refuted, 2 rejected during execution with proof, 1 no-change (documented design):
    • F010 refuted — branch-protection contexts match job names, not workflow display names; infra-bootstrap-validate's job is explicitly named tofu fmt + validate, so no ambiguity exists.
    • F021 refutedcore → features imports violate no repo contract (import-boundary table + eslint plugins say nothing about it); architecture opinion, not a defect.
    • F017 rejected — knip is green: its package.json script parser covers src/templates/email/build.ts, and the src/**/index.ts globstar covers src/index.ts.
    • F015 rejectedapps-api-openapi-drift.yml runs on every PR and triggers on both drift sources (apps/api/src/** and schema.d.ts); UI-only changes cannot create drift.
    • F007 no-change — fail-open rate limiting on Valkey errors is documented intent with Traefik edge limiting as the explicit backup layer.

Security

  • F002 — new prod env invariant: CACHE_ENABLED requires CACHE_PROVIDER=valkey (JWT revocation state must survive restarts / propagate across replicas). The prod compose api service never set the var — it silently inherited the per-process memory default; now pinned. SECURITY.md documents it.
  • F001 — JWT revocation cache-failure policy made explicit and configurable: JWT_REVOCATION_FAIL_CLOSED (default false; exposure bounded by the 15-min JWT TTL, while fail-closed would turn any Valkey blip into a global auth outage). Both check paths log failMode; SECURITY.md lists the events to alert on.
  • F003join-requests.approve() UPDATE now filters by accountId, matching deny(). Defense-in-depth only (the transaction's SELECT was already scoped); cross-account approve test added.
  • F012 + F013 — Stripe webhook plan mutations (checkout.session.completed, subscription upsert/delete) now write stripe.reconciled audit rows as a system actor with resource account:{id}. record() persists the previously-never-written targetAccountId column, and all entity-resource account events (billing checkout/portal, invitations, join requests) set it — these were invisible to GET /accounts/:id/audit-log.

Guardrails (lint as a contract)

  • F004pre-push-ci-parity silently no-oped in both apps (the audit only caught ui): ui's manifest had a stale stages shape; api's pointed at .github/workflows/ci.yml which exists nowhere. The rule now resolves the workflow by walking up to the monorepo root and fails closed on malformed manifests / unresolvable workflows. RED proven live in both apps before the manifest fixes.
  • F014 — the audit's specific claim was refuted (no setup-node usage anywhere), but the adjacent gap was real: 10 workflow bun-version: pins had no guard. engine-pin-parity now scans the resolved monorepo workflows and checks every pin against packageManager.
  • F022 — stale eslint override (tests/auth/role-schema-parity.test.ts) removed + new eslint-override-paths-exist lint-meta rule: literal test paths in eslint.config.* must exist on disk. RED→GREEN demonstrated live.
  • F020eslint-plugin-structured-logging 0.1.1→0.1.2 in ui (lockfile refreshed); shared-tool-version-parity now matches the whole @boring-stack-pkg/ scope by prefix, so any first-party plugin declared in two apps must stay in lockstep. The extension surfaced exactly this drift as RED before the bump.
  • F005 (docs) — new scripts/check-fragments.mjs verifies every intra-site #fragment against built HTML ids and runs as the last build:ci step. lychee --include-fragments was evaluated and rejected: it false-positives on pretty-URL directory links (8 of 9 errors were false). The new gate caught a second genuinely dead anchor the audit missed (/api/acl/#status-driven-features#feature-gates).

CI / infra

  • F011prod-image-build now triggers on apps/api/** / apps/ui/** (the prod Dockerfiles COPY . ., so the whole app tree is image input; previously only Dockerfile/lockfile edits triggered it).
  • F018 — infra pre-push yamllint globbed infra/compose/.github/workflows/*.yml (matches nothing) — workflow YAML was silently unlinted locally. Now mirrors the CI yamllint job exactly. Full 3-stage gate run locally: green.
  • F006 + F019 (docs) — Cloudflare build documented as build:ci (docs-data freshness + fragment gate; sibling defaults resolve without env vars) and .nvmrc 22→24 to match engines.node >=24. Operator action: update the CF Pages dashboard to match (build command + Node version).

Tests

  • F009 — MFA lockout branches (mfa.routes.ts 57–61 / 100–104, previously DA:0) covered: max-attempts lockout on both verify-login and verify-recovery, no cookies issued, consumed challenge reports expiry.
  • F008QueueManager (0/7 functions covered) was untestable by construction: its input demanded concrete worker classes (private fields defeat structural stubs; the merge bar bans casts). It now declares the minimal structural surface it touches (IManagedQueue / IEnqueueableQueue / IManagedWorker) — real BullMQ instances satisfy these implicitly — and 8 stub-based unit tests cover all enqueue paths (retry envelopes, web-push null no-op), getStats, and close(), with no Valkey dependency. setupQueues boot stays covered by full-stack-smoke e2e; its inline-fallback accessor contract is pinned.
  • F016 — AGENT_CONTRACT no longer oversells bun run validate: push/CI additionally enforce the coverage ratchet + dist build. Folding test:coverage into validate was measured and rejected — the 70% function floor fails without the integration stack (68.61% with DB-skipped tests).

Validation

  • bun run check green in api + ui; check:docs-data green; docs build:ci (incl. new fragment gate) green
  • api suite vs live integration DB: 1096 tests, 0 fail (2 skip)
  • compose config validated across every overlay combo (infra pre-push, all 3 stages)
  • Pre-push full-stack smoke gate ran at push time

Follow-ups (not in this PR)

  • CF Pages dashboard: build command → bun run build:ci, Node 24
  • boringstack-xyz/eslint-plugins: extend audit-log mutatingPrefixes with ^handle so webhook handlers are plugin-covered (instances fixed here)

Full decision log: .audit/execution-summary.json (local, gitignored).

agjs added 17 commits June 2, 2026 19:41
… workflows

Both apps' rules silently no-oped: ui manifest had a stale stages-only shape and both rules required an app-local .github/workflows that does not exist in the monorepo (api pointed at ci.yml which exists nowhere). The rule now walks up to the nearest directory containing the manifest's ciWorkflow, flags malformed manifests, and flags unresolvable workflows instead of skipping.

Audit: F004
… bun-version pins

The workflow pin check only looked at the app-local .github/workflows, which is absent in a monorepo checkout, so it never scanned the workflows that actually run. It now resolves the nearest .github/workflows via resolveWorkflowsDir and additionally verifies every workflow bun-version pin against package.json packageManager — previously 10 unguarded pins. Note: the audit's claimed node-pin drift was refuted (no setup-node usage in any workflow); the bun-pin gap was the real exposure.

Audit: F014
…exist rule

tests/auth/role-schema-parity.test.ts was referenced by the test-file-mirrors-source override but does not exist. New lint-meta config rule fails when any literal test path quoted in eslint.config.* is missing on disk, so dead overrides can no longer accumulate.

Audit: F022
approve()'s UPDATE filtered only id+pending while deny() also filtered accountId. The transaction's SELECT is accountId-scoped so this was not exploitable, but the UPDATE now matches deny() for defense-in-depth. Adds a cross-account approve test asserting notFound and that the row stays pending.

Audit: F003
…nabled

CACHE_PROVIDER silently defaulted to the per-process memory provider, splitting JWT revocation state (logout, password-reset session kill, per-jti blocklist) across replicas and losing it on restart. New env invariant fails boot loud; prod compose api service now pins CACHE_PROVIDER=valkey; SECURITY.md documents the requirement. Explicit accept/reject tests added; CACHE_ENABLED=false remains a valid single-replica opt-out.

Audit: F002
apps/docs/.nvmrc pinned Node 22 while package.json engines requires >=24 (matching api/ui). Cloudflare Pages reads .nvmrc, so production builds ran on a Node major the repo does not support. DEPLOY.md Node row updated.

Audit: F019
…ring-stack-pkg parity

ui pinned 0.1.1 while api pinned 0.1.2, so the two apps enforced structured logging from different plugin releases. shared-tool-version-parity now also matches the @boring-stack-pkg/ scope by prefix, so any first-party plugin declared in two apps must stay in lockstep. Rule surfaced the drift (RED) before the bump (GREEN); ui bun.lock refreshed.

Audit: F020
…ragments

CostCalculator was defined but rendered nowhere while the changelog and cost-methodology copy advertised it on Why BoringStack — mounted in Running costs (client:visible), so /architecture/why-boringstack/#cost-calc-title resolves. New scripts/check-fragments.mjs verifies every intra-site #fragment against built ids (lychee --include-fragments false-positives on pretty-URL directory links) and runs as the last build:ci step; it also caught and fixed a second dead anchor: /api/acl/#status-driven-features -> #feature-gates.

Audit: F005
DEPLOY.md documented the CF Pages build as bare `bun run build`, which skips the docs-data freshness check — production could serve stale src/data catalogs after api/ui lint-meta changes. build:ci runs the check (sibling-app defaults resolve in the monorepo checkout without extra env vars) plus the new fragment-link gate. Operator action: update the build command in the CF Pages dashboard to match.

Audit: F006
…ntId

Webhook handlers (checkout.session.completed, subscription upsert/delete) mutated accountPlans with no audit trail — they now record stripe.reconciled with resource account:{id} as a system actor, putting the previously-unused STRIPE_RECONCILED constant to work. record() also persists the targetAccountId column (schema had it; the write path never set it), and every account-scoped event whose resource is an entity id (billing checkout/portal, invitations, join requests) now sets it — those events were invisible to GET /accounts/:id/audit-log. New listForAccount tests cover both match conventions.

Audit: F012, F013
…_CLOSED)

Revocation checks failed open on cache errors by documented design. Decision (north star: security with availability eyes-open): keep fail-open as the default — exposure is bounded by the 15-minute JWT TTL, and fail-closed would turn any Valkey blip into a global auth outage — but make the policy explicit and configurable. JWT_REVOCATION_FAIL_CLOSED=true rejects authenticated requests on cache errors for strict deployments. Both check paths log their failMode; SECURITY.md documents the tradeoff and the events to alert on. Cache-error tests added for the default path, env-wiring tests for the flag.

Audit: F001
The yamllint stage globbed infra/compose/.github/workflows/*.yml, which doesn't exist — the glob expanded to nothing and workflow YAML was silently unlinted locally while CI linted repo-root .github/workflows/. The stage now mirrors the CI yamllint job exactly: same relaxed config, same targets. Full pre-push gate run locally: all 3 stages pass.

Audit: F018
Dockerfile.prod runs COPY . . for both apps, so the entire app tree is image input — but the dorny filter and on.push paths only listed Dockerfiles, lockfiles, and package.json. App-only source changes could merge without the prod image sanity build. Filters now use apps/api/** and apps/ui/**.

Audit: F011
…date

The contract sold bun run validate as the complete merge bar while pre-push and CI additionally enforce the coverage ratchet and the dist build. Decision: document rather than fold test:coverage into validate — the ratchet's 70% function floor fails without the integration stack (verified locally: 68.61% with DB-skipped tests), so putting it in validate would break the fast local gate.

Audit: F016
…very

The locked_out branches (mfa.routes.ts 57-61 / 100-104) had zero coverage. New route tests drive MFA_MAX_CHALLENGE_ATTEMPTS wrong codes through both endpoints, assert the lockout 401 with no cookies issued, and that the consumed challenge reports expiry afterwards. Validated against the live integration DB: 9 pass.

Audit: F009
…nterfaces

QueueManager was untestable by construction: its input demanded concrete worker classes (private fields defeat structural stubs, and the merge bar bans casts). It now declares the minimal surface it touches — IManagedQueue / IEnqueueableQueue / IManagedWorker — which real BullMQ instances satisfy implicitly. New unit tests cover all enqueue paths (retry envelopes incl. web-push null no-op), getStats (count defaulting, optional web-push), and close() lifecycle, with no Valkey dependency so they count in every coverage run. setup-queues boot stays covered by full-stack-smoke e2e; its inline-fallback accessor contract is pinned by a new test. Full suite: 1096 tests, 0 fail.

Audit: F008
@agjs
agjs merged commit c9d0295 into main Jun 2, 2026
28 checks passed
@agjs
agjs deleted the chore/audit-fixes-20260602-1932 branch June 2, 2026 21:08
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