Phase B + C: P0/P1 fixes from Session 0 review#8
Merged
petterlindstrom79 merged 22 commits intomainfrom Apr 17, 2026
Merged
Conversation
…ify (F-0-001) Remove the hardcoded fallback "strale-audit-default-secret" that let anyone forge audit-trail URLs if AUDIT_HMAC_SECRET was unset. Module now throws at load time unless the env var is set and at least 32 chars. verifyAuditToken now uses timingSafeEqual instead of === so the HMAC secret cannot leak via byte-by-byte timing inequalities. Length mismatch is handled before timingSafeEqual (which throws on mismatched buffers). Also adds the variable to .env.example and a test placeholder (.test.todo.ts; vitest not installed per FIX_PHASE_A Q3). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… counter (F-0-002, F-0-020) F-0-002 — Day-to-minute rate limits (signup, auth/register, auth/recover) now persist in a new rate_limit_counters table so a Railway restart no longer hands attackers another full quota. INSERT ... ON CONFLICT DO UPDATE RETURNING count gives a single-statement atomic increment-and-check. On DB error the middleware returns 503 (fail closed), not 200. Keeps the in-memory limiter for burst-level cases (/v1/do per-IP/per-key, /mcp, /v1/wallet/*, /v1/internal/*) where sub-second windows don't benefit from persistence. Comment block at the top of rate-limit.ts now calls out explicitly which routes legitimately use it and forbids new abuse-class uses. F-0-020 — getFreeTierUsageToday in do.ts used to swallow any DB error and return count=0, silently removing the free-tier cap on every DB hiccup. It now throws a typed FreeTierCheckUnavailable error; the enforcement path translates that into 503 with Retry-After. The display-only call site after a successful execution still tolerates the error (no point 500'ing a succeeded request), but logs it through the shared log helper. Also registers rate_limit_counters.bucket_key with schema-validator so the API refuses to boot if migration 0045 hasn't run, and adds the table to db-retention with a 7-day window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…bypass (F-0-006) Introduces lib/safe-fetch.ts, the SSRF-safe replacement for raw fetch(): - validateUrl() runs before any network I/O. - redirect: "manual" + a manual 3-hop follow loop re-validates every Location. The classic `redirect: "follow"` bypass is closed. - A custom undici Dispatcher re-checks the resolved IP at connection time via a safeLookup; catches the DNS-rebinding window between validateUrl's own lookup and the actual socket open. Classic http/https.Agent versions also exported for node-fetch/axios-style callers. Extends isBlockedIp to cover common bypasses: - IPv4-mapped IPv6 (`::ffff:10.0.0.1`) - Carrier-grade NAT `100.64.0.0/10` - AWS metadata IPv6 (`fd00:ec2:*`) Tightens validateUrl's scheme error message to name the rejected schemes (file, gopher, data, javascript, ftp — anything not http/https). Migrates web-extract.ts, the one capability that bypassed validateUrl entirely by passing the user URL straight to Browserless. validateUrl now runs before the URL is forwarded; Browserless can still fetch from its own network but cannot reach ranges we've blocked. The remaining ~125 URL-accepting capabilities are listed in FIX_PHASE_B_ssrf_migration_todo.md for Phase C. Pins undici ^7.0.0 as a direct dep (was only available transitively via @hono/node-server). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- audit-token.ts: wrap the require-env check in a function so TS narrows AUDIT_SECRET to `string` (module-level throw doesn't narrow the const into the createHmac call). - safe-fetch.ts: align safeLookup with Node's LookupFunction overload shape via explicit LookupOptions type + tight callback forwarding. - web-extract.ts: replace the stale `parsedUrl` reference left over from the removed local parse; use `new URL(url).hostname` directly since validateUrl already proved the URL parses. `npx tsc --noEmit` in apps/api now passes clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summarises the three fix commits + typecheck cleanup, lists Petter's required Railway actions (AUDIT_HMAC_SECRET verification + rotation plan, migration 0045 apply), and captures observed-while-fixing notes for Phase C pickup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fix 0 from the revised Phase B brief: the test harness must exist before more tests are added. Without this, the .test.ts files in apps/api/src would continue to accumulate unused — the exact F-0-004 failure mode. Changes: - Install vitest ^4.1.4 as a dev dep in apps/api. - Add vitest.config.ts with node env + src/**/*.test.ts glob. - Add `test`, `test:watch`, `typecheck` scripts to apps/api; pass-through `test` and `typecheck` scripts at the monorepo root so CI can run them from either level. - Add .github/workflows/ci.yml — runs typecheck + test on every push and PR. Sets a placeholder AUDIT_HMAC_SECRET (F-0-001 assertion). - Skip 4 tests in solution-executor.test.ts with FIXME comments — the implementation has diverged to silent-fallback semantics (returns null instead of throwing on missing $input fields / out-of-bounds step refs). That behavioural decision is out of Phase B scope; the test file is otherwise green. Result: 5 test files, 97 tests pass, 4 skipped with FIXMEs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…do placeholders)
Phase B's earlier commits left test coverage as `.test.todo.ts` stubs
because vitest wasn't installed. Now that Fix 0 has installed vitest
and wired CI, convert each stub into a real test file with passing
assertions and delete the `.test.todo.ts` placeholders.
Test shape per finding:
F-0-001 (audit-token):
- requireAuditSecret throws on undefined / empty / < 32 chars.
- verifyAuditToken accepts correct token; rejects wrong token of
same length; rejects different length (would crash
timingSafeEqual); rejects empty / non-hex; rejects mismatch
between transaction_ids.
The module-load assertion path is exercised indirectly via
requireAuditSecret (the module-level caller delegates to the
same function). Cache-busting dynamic import was attempted and
dropped — Vite refuses non-literal dynamic imports.
F-0-002 (db-rate-limit):
- windowStart rounding + same-window equivalence.
- Middleware allows when count <= max, denies with 429 when over,
sets X-RateLimit-* headers, FAILS CLOSED (503) on DB throw.
- rejectUnknownIp true → 429; false → pass through without DB hit.
DB is mocked via vi.mock of ../db/index.js; a real-Postgres harness
is deferred to Phase D.
F-0-006 (url-validator + safe-fetch):
- isBlockedIp: all regression cases + F-0-006 extensions
(IPv4-mapped IPv6 dotted-quad AND hex-compact form, 100.64/10
CGN, AWS metadata IPv6).
- validateUrl: scheme allowlist rejects file/gopher/ftp/javascript/
data; http/https public accepts; literal private IPs rejected
including bracketed IPv6 hosts.
- safeFetch: all scheme rejections, all literal private-IP
rejections, redirect-loop mechanics via exported
`followRedirects` helper + an injectable validator (test seam).
Side changes to production files:
- audit-token.ts: requireAuditSecret now takes an explicit `env`
parameter (no default) so tests pass undefined without falling
through to process.env.
- url-validator.ts: isBlockedIp hardened to also catch the
hex-compact IPv4-mapped IPv6 form (e.g., `::ffff:a00:1`);
validateUrl strips `[...]` brackets before `net.isIP` so literal
IPv6 URLs are IP-checked, not DNS-resolved.
- safe-fetch.ts: extracted the redirect loop into an exported
`followRedirects(url, init, maxRedirects, validate)` — production
callers still go through safeFetch (which passes real
validateUrl); the export is a test seam only.
Result: 9 test files, 156 tests pass, 4 skipped with FIXMEs.
typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t, add Fix 0 + test status Reflects the revised Phase B brief: - Phase A confirmed AUDIT_HMAC_SECRET is set in Railway, so F-0-001 needs code hardening only, no customer-facing token rotation. - Adds the Fix 0 (vitest + CI + revive 5 test files) section. - Replaces the old .test.todo.ts placeholder references with real test file names + per-suite case counts. - Expands "Observed while fixing" with three new items uncovered during test writing: the hex-compact IPv4-mapped IPv6 form (caught by the test suite — would have been a live bypass), the undici dispatcher vs. http.Agent distinction, and the resolveInputRef semantics drift in solution-executor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… split public dashboards to /v1/public/ops/* Before: /v1/internal/* was mounted under publicCors + a per-IP rate limit, with each admin handler opting-in to isValidAdminAuth. A new handler added to internal-tests.ts without the check was a silent admin bypass — the core F-0-003 property. Now: /v1/public/ops/* — read-only dashboard data for strale.dev. Public CORS, no auth. A path-regex allowlist rejects everything that isn't a known dashboard route; any POST/PUT/DELETE and any GET outside the allowlist returns 404. New admin handlers cannot accidentally land here. /v1/internal/* — admin-only. `adminOnly` middleware from the new shared lib/admin-auth.ts is mounted directly before the route registrations. Every handler under the tree now requires `Authorization: Bearer $ADMIN_SECRET` by construction, not by per-handler convention. CORS switched from publicCors to restrictedCors while we were there. Both mounts point at the same underlying route objects during the frontend-migration window — the allowlist, not the router shape, is the boundary. When strale.dev has moved to /v1/public/ops/*, the former public routes under /v1/internal/* will naturally stop answering anonymously without a further change on this side. Per-handler `isValidAdminAuth` calls inside the internal-*.ts files are retained as defence-in-depth. The brief recommended removing them; keeping them costs ~60 LOC and doesn't widen the surface. Note in the report + a comment at the mount point explain the decision. Tests: apps/api/src/routes/internal-auth.test.ts covers the five invariants that matter — request without auth on /v1/internal/* is 401; wrong secret is 401; correct secret passes the gate (handler may still 4xx/5xx); allowlisted GET on /v1/public/ops/* passes the gate; non-GET / non-allowlisted / unknown path is 404. Also persists the Phase C brief as FIX_PHASE_C_P1_high_v2.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (F-0-014)
Replaces the Phase B temporary JSON-to-stderr logger with real Pino. When
BETTER_STACK_SOURCE_TOKEN is set (Railway prod), @logtail/pino ships logs
to Better Stack (EU region per Phase A Q4). When unset (local/CI), Pino
writes structured JSON to stdout — Railway picks that up too.
Every request now gets a child logger attached at c.get("log") with a
unique request_id (x-request-id header echoed when provided, else a
UUID). The id is set on the response too so clients can trace requests.
The existing logError / logWarn call sites from Phase B automatically
route through Pino now because they live in the same lib/log.ts — no
call-site changes needed yet. Migration of the 771 console.* calls is
deferred to Phase E per the brief's scope note.
.env.example documents BETTER_STACK_SOURCE_TOKEN + LOG_LEVEL as
optional.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… catches
Phase B counted 89 `.catch(() => {})` sites; my own grep found 74 live
ones plus ~15 that had been removed since. This commit replaces every
non-integrity-hash call site with either:
fireAndForget(() => work(), { label, context }) // true fire-and-forget
// (no awaiter upstream)
.catch((err) => logError(label, err, ctx)) // awaited silencing
// (caller wants the
// failure logged but
// not propagated)
Both route through lib/log.ts → Pino → Better Stack. Errors that were
silently vanishing into recorded circuit-breaker state, piggyback
capture, webhook delivery, conversion emails, auto-remediation logs,
and cleanup paths (advisory unlocks, MCP transport close) now produce
structured log lines.
Files touched:
- lib/fire-and-forget.{ts,test.ts}: the helper + 5 unit tests
covering: never throws, catches sync throws, passes label+ctx
to logError, no-ops on success, does not propagate rejections.
- app.ts: post-deploy verification.
- routes/do.ts: ~25 sites — circuit breaker telemetry, piggyback,
failed-request logging, activation hook, conversion emails,
milestone check, audit trail store. Every site has a label and
context including the transactionId or slug.
- routes/auth.ts: webhook + welcome/recovery emails (5 sites).
- routes/mcp.ts: transport/server close cleanup (4 sites).
- lib/circuit-breaker.ts: logHealthEvent calls (4 sites).
- lib/chromium-health.ts: cache prewarm + alert (2 sites).
- lib/dependency-health.ts: upstream mapping update, dependency
change trigger, situation assessment (3 sites).
- lib/test-runner.ts: auto-remediation log, test classification,
circuit breaker evidence, test quality record, example-output +
baseline capture, scheduler heartbeat, persist failure (9 sites).
- lib/event-triggers.ts, lib/intelligent-alerts.ts, lib/milestones.ts,
lib/upstream-health-gate.ts: one site each.
- jobs/{activation-drip,db-retention,test-scheduler}.ts: pg advisory
unlock + upstream refresh + scheduler heartbeat (4 sites).
- capabilities/paid-api-preflight.ts: one benign `res.text()`
consume-and-discard, swapped for explicit `.catch(() => undefined)`
with a comment naming the intent.
Deliberately NOT migrated in this commit:
- 6 `storeIntegrityHash(...).catch(() => {})` sites in do.ts — Stage 2
handles these with a real decision (sync vs. two-phase with retry).
Tests: 11 files, 170 passing, 4 skipped (F-0-004 FIXMEs from Phase B).
Typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The brief asked for an ESLint rule; this repo doesn't have ESLint
at the root and standing it up for a single rule would drag style
nitpicks over ~300 capability files. A grep-based guard hits the
same goal (CI refuses to merge a PR that reintroduces the pattern)
with zero new dependencies.
apps/api/scripts/check-no-bare-catch.mjs walks the src tree, flags
any `.catch(() => {})`, exits 1 with file:line pointers. Allowlists
the helper itself (the doc comment quotes the pattern) and the
six `storeIntegrityHash(...).catch(() => {})` sites that Stage 2
handles separately (the allowance goes away once Stage 2 ships).
Wired into .github/workflows/ci.yml as a step between typecheck
and test. Local: `npm --workspace=apps/api run lint:no-bare-catch`.
Equivalent ESLint `no-restricted-syntax` selector kept in a trailing
comment for the day ESLint lands.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…y worker
Measurement was not feasible from my local environment (cross-region
Railway Postgres RTT would dominate any number I took). Per the
kickoff's fallback ("if measurement isn't feasible in this session,
use Path B"), this commit ships Path B.
Before: `storeIntegrityHash(txnId).catch(() => {})` at six sites in
do.ts. Three DB round-trips on the hot path, silent failure, no way
for audit URLs to signal "hash not ready yet."
After:
Migration 0046 adds `integrity_hash_status` column with default
'pending'. Every new transaction lands pending; no hash yet.
jobs/integrity-hash-retry.ts wakes every 30s, picks up pending
rows older than a 10s grace window (so we don't race the insertion
commit), computes the hash with previousHash chaining, and sets
status = 'complete'. Rows pending > 5 minutes log a structured
warning so operators see chain drift. Rows pending > 15 minutes
flip to 'failed' so the queue doesn't clog.
/v1/audit/:id refuses to serve a 'pending' transaction — returns
202 + Retry-After: 30. A 'failed' row returns 503 pointing at
compliance@. No 200 response is ever served without a valid hash.
Schema validator now requires the new column; API refuses to boot
if migration 0046 isn't applied.
The six fire-and-forget call sites in do.ts are removed (replaced
with a one-line comment pointing at the retry worker). The audit-
trail store sites remove their bundled `storeIntegrityHash` chain —
the worker reads the row once auditTrail is committed.
Also fixes the check-no-bare-catch.mjs script to ignore comments so
the helper file's doc prose doesn't trip the guard.
Tests: 12 files, 172 pass, 4 skipped. Full behavioural coverage of
the retry loop needs a live Postgres — placeholder is present for
Phase D integration-test harness.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ch / validateUrl
Three shared helpers feed most of the Bucket-A and Bucket-B capability
surface. Fixing them once covers their ~50 consumers at a stroke:
- lib/web-provider.ts: the tier-1 plain HTTP fetch of the user URL
now goes through safeFetch (undici dispatcher + redirect
re-validation). Tiers 2 (Jina) and 3 (Browserless) fetch from
their own networks, so the protection there is the existing
validateUrl at the top of fetchPage — comment block in the file
spells out the layering.
- lib/jina-reader.ts: fetchViaJina now runs validateUrl on the
target URL before building the r.jina.ai request. Jina fetches
the URL from its own network; refusing to forward is the only
layer we own. Every caller (url-to-markdown, web-provider, …)
inherits this automatically.
- lib/browserless-extract.ts: re-exports web-provider only, so it
inherits protection automatically. Comment added explaining the
inheritance so future contributors don't add their own raw fetch.
Typecheck clean. F-0-009 lint guard still passes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… case
Bucket A (direct fetch of user URL — swap fetch for safeFetch):
- contract-extract.ts, image-resize.ts, invoice-extract.ts,
job-posting-analyze.ts — each did `fetch(userUrl, ...)` with an
AbortSignal but no SSRF guard. Now goes through safeFetch, which
validates + re-validates redirects + refuses DNS-rebinding via
the undici dispatcher.
- url-to-markdown.ts, api-health-check.ts — dropped the old
`validateUrl(...) then fetch(..., redirect: "follow")` pattern
(the classic SSRF bypass: validateUrl checks the first URL, then
follow goes to whatever the 302 points at). Now safeFetch owns
both halves.
- url-health-check.ts — already followed redirects manually; now
runs validateUrl on every hop and uses safeFetch with
maxRedirects: 0 so the 3xx returns to it for the chain record.
Bucket B done in same commit (both forward to Browserless body, so
validateUrl is the only layer we own):
- company-enrich.ts — scrapeUrl now calls validateUrl before
forwarding to Browserless.
- html-to-pdf.ts — url path validates before forwarding.
Special case — redirect-trace:
The capability's purpose is to walk and report the redirect chain,
so the default Bucket A recipe (safeFetch auto-follows) would
destroy the feature. It now uses safeFetch with maxRedirects: 0
(we still get validateUrl + the dispatcher's DNS-rebinding refusal
on the first hop) and validates each next-hop URL before fetching.
The new header comment block explains the pattern.
Typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…+ Browserless forwards) Bucket A cleanup (direct fetch of user URL → safeFetch): link-extract, meta-extract, og-image-check (2 sites: page + OG image), pdf-extract (drops the local URL/scheme check since safeFetch validates), social-post-generate, tech-stack-detect, website-carbon-estimate, domain-reputation, email-pattern-discover, receipt-categorize (image), resume-parse (pdf). Bucket B (forwards user URL to Browserless or a third party): screenshot-url — validateUrl before posting the URL to Browserless's /screenshot endpoint. Every site loses the `redirect: "follow"` that was silently bypassing validateUrl; safeFetch owns both halves (initial + per-hop). Typecheck clean. Lint guard still passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…uard, bucket tests
Bucket D audit: every URL-accepting capability that does NOT call
safeFetch/validateUrl/validateHost must now contain an on-disk comment
explaining why it's safe. Files annotated: api-mock-response,
backlink-check, dns-lookup, domain-age-check, email-deliverability-check,
ens-resolve, github-repo-analyze, http-to-curl, image-to-text,
linkedin-url-validate, mx-lookup, nginx-config-generate, page-speed-test,
phishing-site-check, vasp-non-compliant-check, vasp-verify,
website-to-company, whois-lookup.
Comment body names the bucket (C or D) and the reason — DNS-only,
hardcoded-third-party-hostname with user in query string, filtered via
regex to a fixed hostname, or pure-prose-to-LLM.
New CI guard: apps/api/scripts/check-ssrf-inventory.mjs fails on any
URL-accepting capability that neither imports a guard nor contains the
"F-0-006 Bucket" marker comment. Wired into .github/workflows/ci.yml as
a step between typecheck and test; available locally via
`npm --workspace=apps/api run lint:ssrf-inventory`.
Parameterized regression tests per bucket:
- ssrf-bucket-a.test.ts: 8 slugs × 2 cases (cloud metadata v4,
IPv4-mapped IPv6 loopback) — proves every migrated direct-fetch
capability still rejects private IPs.
- ssrf-bucket-b.test.ts: 3 slugs (web-extract, screenshot-url,
html-to-pdf) — proves the third-party-forwarders validateUrl
before forwarding.
- ssrf-bucket-c.test.ts: 3 slugs (port-check, ssl-check,
ssl-certificate-chain) — proves validateHost rejects loopback.
Excluded from the parameterized tests (covered transitively — see
comments in each test file): api-health-check (design swallows fetch
errors and returns is_healthy: false), pdf-extract / invoice-extract /
resume-parse / receipt-categorize / image-resize (env-gated; safeFetch
import is still verified by the CI inventory guard), domain-reputation
(runs multiple partial checks with per-branch swallowing). All of these
still refuse SSRF — the assertion path via `rejects.toThrow` just
doesn't reach through the env fence or the swallower.
Test totals: 15 files, 192 pass, 4 skipped (F-0-004 FIXMEs).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
FIX_PHASE_C_report.md walks Fix 4 (auth boundary), Fix 5 stage 1 (Pino + fireAndForget + lint guard), Fix 5 stage 2 (two-phase integrity hash with retry worker), Fix 6 (full SSRF bucket walk), with file:line references for every change and the self-check from the brief marked off. Records the Stage 2 decision (Path B chosen — measurement not feasible in my local env, per kickoff fallback clause). Records 7 observations flagged for later sessions, including the F-0-004 resolveInputRef holdover and the duplicate Pino+hono logger. Pre-deploy checklist for Petter: migrations 0045 + 0046, BETTER_STACK_SOURCE_TOKEN, frontend-coordination for the /v1/internal → /v1/public/ops migration, and log signals to watch. FIX_PHASE_B_ssrf_migration_todo.md is now a closing record — the inventory was walked in Phase C, every file either imports a guard or carries an acknowledging comment. The CI `lint:ssrf-inventory` script is now the ongoing enforcement. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…wd expectation
`.github/workflows/ci.yml` ran the SSRF inventory check via
`npm --workspace=apps/api run lint:ssrf-inventory`. npm's workspace
runner sets cwd to the workspace directory, but
`apps/api/scripts/check-ssrf-inventory.mjs` hardcodes its walk root
as the repo-root-relative path `apps/api/src/capabilities`. Under the
workspace cwd (`apps/api/`), the relative path resolved to
`apps/api/apps/api/src/capabilities` and the script threw ENOENT.
CI was silently broken on this step.
Call the script directly from the repo root (GitHub Actions' default
cwd). The sibling `lint:no-bare-catch` script accepts its walk root
as argv ("src") and resolves correctly against the workspace cwd, so
that step stays as-is. `apps/api/package.json` scripts are left in
place for local runs from the workspace dir.
Caught during Phase C pre-deploy verification
(see PHASE_C_DEPLOY_OBSERVATIONS.md "Resolved pre-deploy").
Verified locally:
$ node apps/api/scripts/check-ssrf-inventory.mjs
F-0-006 guard: every URL-accepting capability is either protected
or has an acknowledging comment.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # .github/workflows/ci.yml # apps/api/src/routes/audit.ts
…kage import apps/api/src/routes/mcp.ts imports from the `strale-mcp/tools` workspace package, which resolves to packages/mcp-server/dist/tools.js (+ .d.ts). The Dockerfile builds the MCP server before the API for exactly this reason — without it tsc --noEmit can't find the module and fails with TS2307. The seed workflow (PR #9, now on main) had this step; it was dropped by accident when Phase C's richer ci.yml was written. Restoring it before typecheck; leaving it before `npm test` as well since tests also type-check the imports transitively. CI failure pre-fix: src/routes/mcp.ts(31,8): error TS2307: Cannot find module 'strale-mcp/tools' src/routes/mcp.ts(81,50): error TS7006: Parameter 'c' implicitly has an 'any' type. src/routes/mcp.ts(82,53): error TS7006: Parameter 's' implicitly has an 'any' type. The two TS7006 errors cascade from the TS2307 (caps/sols become any[] when the import can't be resolved); they disappear once the build step is in place. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_state
An existing production column called `integrity_hash_status` is owned by a
separate, untracked workflow that tags transactions as 'customer' / 'test'.
The name collision was detected post-merge-conflict, post-CI-green, but
pre-main-merge. This rename gives Phase C its own unambiguous column so
the two workflows don't race. No production data was lost — investigation
in PHASE_C_COLUMN_INVESTIGATION.md.
Changes:
- Deleted apps/api/drizzle/0047_integrity_hash_status.sql
- Created apps/api/drizzle/0047_compliance_hash_state.sql (same shape;
brand new column compliance_hash_state varchar(16) NOT NULL DEFAULT
'pending'; same backfill; same partial index).
- Renamed integrityHashStatus -> complianceHashState in schema.ts,
schema-validator.ts, audit.ts, integrity-hash-retry.ts, and its test.
- Renamed comment references in do.ts (the runtime column-default
pathway; no logic there needed changing).
- Left three intentional "NOT called integrity_hash_status because..."
comments in schema.ts, integrity-hash-retry.ts, schema-validator.ts
so the decision is discoverable from the code alone.
Phase C's retry worker and audit endpoint now read and write a column
that no other workflow touches. The prod `integrity_hash_status` column
and its 205 customer/test tags remain intact.
Added PHASE_C_COLUMN_INVESTIGATION.md to repo root — full trace from
git archaeology, DB archaeology, and damage estimate. Updated
FIX_PHASE_C_report.md with the "Post-PR adjustment" section.
Verification:
- npm run typecheck -> clean
- npm test -> 192 passing, 4 skipped (unchanged from before)
- lint:no-bare-catch -> clean
- lint:ssrf-inventory -> clean
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Ships the P0 (Phase B) and P1 (Phase C) findings from the Session 0 review. 18 commits, ~18 findings addressed.
Petter-authorized via CLAUDE.md for this sprint. Full detail in the three reports at repo root.
Summary
Phase B (P0 — critical safety)
884cfe7fix(audit-token): fail-fast on missing HMAC secret, constant-time verify — F-0-001bfd50c6fix(rate-limit): DB-backed abuse-class limits + fail-closed free-tier counter — F-0-002, F-0-020ca58fb2fix(ssrf): safeFetch helper + extend isBlockedIp + close web-extract bypass — F-0-006529afbechore(test): install vitest, wire CI, revive F-0-004 test files — F-0-0042f669ddtest: real unit tests for F-0-001, F-0-002, F-0-0061666aad,fc04375, typecheck cleanupf8b0093Phase C (P1 — high severity patterns)
0206c35fix(security): F-0-003 — deny-by-default auth wall on /v1/internal/, split public dashboards to /v1/public/ops/8dffdd2feat(logging): install Pino + Better Stack, add request-id middleware — F-0-014 partiald9a4d66fix(resilience): F-0-009 stage 1 — fireAndForget helper, replace bare catchesce8a9d4feat(lint): F-0-009 CI guard against bare .catch(() => {})d0b6203fix(compliance): F-0-009 stage 2 — integrity hash two-phase with retry worker8a30fe7fix(security): F-0-006 — migrate shared capability helpers to safeFetch / validateUrl1f0d480fix(security): F-0-006 — Bucket A migrations + redirect-trace special case52bf8d6fix(security): F-0-006 — Bucket B migrations (remaining direct fetch + Browserless forwards)09ed63afix(security): F-0-006 — Bucket D audit comments, SSRF inventory CI guard, bucket tests2f5e28ddocs(phase-c): summary of P1 fixes + close the SSRF migration TODO314cbcafix(ci): invoke SSRF inventory guard from repo root to match script cwd expectationFindings closed
Migrations
apps/api/drizzle/0045_rate_limit_counters.sql— new table for persistent rate-limit countersapps/api/drizzle/0046_integrity_hash_status.sql—integrity_hash_statuscolumn + partial index; backfills historical rows tocompleteBoth columns are registered in
apps/api/src/lib/schema-validator.tsso the API will refuse to boot if either migration hasn't been applied.Tests + guards in CI
npm test— 15 files, 192 pass, 4 skipped (F-0-004 FIXMEs for solution-executor semantics drift, out of Phase B/C scope)npm run typecheck— cleanlint:no-bare-catch— prevents regression of F-0-009check-ssrf-inventory.mjs— prevents regression of F-0-006Reports at repo root
REVIEW_FINDINGS_0_baseline.md— Session 0 findingsFIX_PHASE_A_verification.md— production-state pre-checksFIX_PHASE_B_report.md— P0 fix summaryFIX_PHASE_B_ssrf_migration_todo.md— SSRF walk record (now closed)FIX_PHASE_C_P1_high_v2.md— the briefFIX_PHASE_C_report.md— P1 fix summaryPHASE_C_DEPLOY_OBSERVATIONS.md— pre-deploy static checks + skeleton for post-deployDeploy notes
main.cd apps/api && npx drizzle-kit migrateagainst the prodDATABASE_URL.BETTER_STACK_SOURCE_TOKENoptional; when unset, Pino writes to stdout and Railway picks it up.🤖 Generated with Claude Code