Skip to content

Commit 823ce16

Browse files
chore(ci): guard against route/method mismatches, record verified mechanics
`npm run check:routes` compares every call in inertia/** and tests/e2e/** against the real route table from `node ace list:routes --json` — not by parsing routes.ts, so group prefixes cannot skew it. Wired into check.sh and the quality workflow. This exists because the bug that started all of this was mechanically detectable and nobody was looking: routes are indexed by method, so a verb mismatch answers 404 rather than 405, which reads like a wrong URL. Verified by temporarily reintroducing the original bug — it was caught, with the line and the reason. CLAUDE.md gains a table of mechanics verified against node_modules rather than recalled: middleware ordering and what is not yet available in the server stack, why a verb miss is a 404, why GET must never be added to csrf.methods, how the Inertia error and redirect contracts work, what FormData cannot express, and the transaction scope of advisory locks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d59eb47 commit 823ce16

5 files changed

Lines changed: 197 additions & 8 deletions

File tree

.github/workflows/quality.yml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,27 @@ jobs:
6767
- name: TypeScript typecheck
6868
run: npm run typecheck
6969

70+
# Catches frontend calls whose verb no route accepts — that mismatch answers 404,
71+
# not 405, so it reads like a wrong URL and is easy to miss in review.
72+
- name: Route/method parity
73+
run: npm run check:routes
74+
env:
75+
TZ: UTC
76+
NODE_ENV: test
77+
APP_KEY: test-app-key-for-ci-only-123456789
78+
DB_HOST: 127.0.0.1
79+
DB_PORT: 5432
80+
DB_USER: sbf
81+
DB_PASSWORD: sbf
82+
DB_DATABASE: sbf_test
83+
SESSION_DRIVER: memory
84+
LOG_LEVEL: error
85+
SMTP_HOST: localhost
86+
SMTP_PORT: 1025
87+
OIDC_ENABLED: 'false'
88+
API_SECRET: test-api-secret
89+
APP_URL: http://localhost:3334
90+
7091
- name: Run unit and functional tests
7192
run: node ace test --no-color
7293
env:

CLAUDE.md

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -171,16 +171,32 @@ await user.load((loader) => loader.load('orders'))
171171
// Role check — role middleware handles this, but in code:
172172
// admin implicitly has supplier access (check middleware/role.ts)
173173

174-
// Method spoofing works ONLY from the query string — never from the body:
175-
// POST /supplier/products/16?_method=PUT ✅
176-
// POST /supplier/products/16 + `_method: 'PUT'` field in the body ❌ 404
177-
// The bodyparser is router middleware, so it runs AFTER route matching —
178-
// `_method` in the body is invisible to the router. From Inertia always issue a
179-
// real router.put()/form.put()/router.delete(); it works with forceFormData too
180-
// (unlike PHP, the Adonis bodyparser parses multipart on PUT/PATCH/DELETE), and
181-
// the Inertia middleware upgrades the 302 redirect to 303 for you.
174+
// Method spoofing is DISABLED (config/app.ts) and must stay that way:
175+
// - `_method` is only ever read from the query string, never the body — the bodyparser
176+
// is router middleware, so it runs AFTER route matching and the body does not exist yet
177+
// - Shield picks whether to validate CSRF from request.method(), so a spoofable verb lets
178+
// a cross-site POST with `_method=GET` in its body skip validation entirely
179+
// From Inertia always issue a real router.put()/form.put()/router.delete(). That works with
180+
// forceFormData too (unlike PHP, the Adonis bodyparser parses multipart on PUT/PATCH/DELETE),
181+
// and the Inertia middleware upgrades the 302 redirect to 303 for you.
182+
// `npm run check:routes` fails the build on any frontend call whose verb no route accepts.
182183
```
183184

185+
### Framework mechanics worth knowing (verified against node_modules, not from memory)
186+
187+
| Mechanism | The rule |
188+
| ------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
189+
| Middleware order | `server.use``router.match``router.use` → named → controller. Server middleware sees no `ctx.route`, `ctx.params`, `request.body()`, `ctx.session` or `ctx.auth`. |
190+
| Verb mismatch | Routes are indexed by method, so a wrong verb returns **404**, never 405. A 404 does not mean the URL is wrong. |
191+
| CSRF gate | Shield validates exactly the verbs in `config/shield.ts`. Never add GET/HEAD/OPTIONS — every navigation would then demand a token. |
192+
| Inertia errors | `errors` must be shared from `inertia_middleware.share()`; without it `form.errors` is always empty, `onError` never fires and `onSuccess` runs after a validation failure. |
193+
| Inertia mutations | A 4xx carrying a `Location` makes the client raise an error modal and swallow the flash. Use a plain 302 plus a flash, and `redirect('back', true)` to keep the query string. |
194+
| `inertia_middleware` placement | Registered inside the session middleware (`router.use`), because its `dispose()` reflashes on a 409 and that only survives if it runs before the session commit. |
195+
| FormData | Everything arrives as a string, and an empty array has no representation at all — the key is simply absent. Serialise "clear all" cases as JSON. |
196+
| Advisory locks | `pg_advisory_xact_lock` releases at commit, so allocation and insert must share one transaction. |
197+
| Throttle keys | Derive from `request.ip()` (honours `trustProxy`), never from the `X-Forwarded-For` header directly. |
198+
| Token lifetime | Remember-me (2y) and API tokens outlive sessions — revoke them when an account is disabled or its password is reset (`#services/credential_revocation`). |
199+
184200
### Vue / Inertia Patterns
185201

186202
```typescript

check.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ else
4646

4747
run_step "Ensure test database exists" node --import=tsx scripts/ensure_test_db.ts
4848
run_step "Test migrations" node ace migration:run --force
49+
# Needs the test env because it boots the app to read the real route table.
50+
run_step "Route/method parity" npm run check:routes
4951
run_step "Unit + Functional tests" node ace test --no-color
5052
run_step "Reset E2E state" npm run test:e2e:reset
5153
run_step "Playwright E2E tests" npm run test:e2e

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
"typecheck:backend": "tsc --noEmit",
2222
"typecheck:frontend": "vue-tsc --noEmit -p tsconfig.frontend.json",
2323
"typecheck:tests": "tsc --noEmit -p tsconfig.tests.json",
24+
"check:routes": "node --import=tsx scripts/check_route_methods.ts",
2425
"generate:app-icons": "node --import=tsx scripts/generate_app_icons.ts",
2526
"generate:keypad-tones": "node --import=tsx scripts/generate_keypad_tones.ts",
2627
"hooks:install": "node -e \"const fs=require('fs'); const cp=require('child_process'); if (!fs.existsSync('.git')) { console.log('[hooks] Skipping install: .git directory not found.'); process.exit(0); } try { cp.execFileSync('git', ['config', '--local', 'core.hooksPath', '.githooks'], { stdio: 'inherit' }); } catch (error) { if (error && error.code === 'ENOENT') { console.log('[hooks] Skipping install: git is not available.'); process.exit(0); } throw error; } console.log('[hooks] Installed hooks path: .githooks');\"",

scripts/check_route_methods.ts

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/**
2+
* Verify that every HTTP call the frontend makes matches a registered route — same verb,
3+
* same path.
4+
*
5+
* This exists because a product-edit form once posted `_method: 'PUT'` in the request body,
6+
* a Laravel idiom that AdonisJS does not honour (the bodyparser is router middleware, so it
7+
* runs after routing). Routes are indexed by method, and a verb miss answers 404 rather than
8+
* 405 Method Not Allowed — so it looked like a wrong URL and shipped to production.
9+
*
10+
* The route table comes from `node ace list:routes --json`, not from parsing routes.ts, so
11+
* group prefixes and named middleware cannot skew it.
12+
*
13+
* Usage: npm run check:routes
14+
*/
15+
16+
import { execFileSync } from 'node:child_process'
17+
import { readdirSync, readFileSync, statSync } from 'node:fs'
18+
import { join, relative } from 'node:path'
19+
import { fileURLToPath } from 'node:url'
20+
21+
const repoRoot = fileURLToPath(new URL('..', import.meta.url))
22+
23+
type Route = { methods: string[]; pattern: string; matcher: RegExp }
24+
type Call = { file: string; line: number; method: string; url: string; source: string }
25+
26+
function loadRoutes(): Route[] {
27+
const raw = execFileSync('node', ['ace', 'list:routes', '--json'], {
28+
cwd: repoRoot,
29+
encoding: 'utf8',
30+
env: { ...process.env, NODE_ENV: process.env.NODE_ENV ?? 'test' },
31+
maxBuffer: 32 * 1024 * 1024,
32+
})
33+
34+
const domains = JSON.parse(raw) as Array<{
35+
routes: Array<{ methods: string[]; pattern: string }>
36+
}>
37+
38+
return domains
39+
.flatMap((domain) => domain.routes)
40+
.map((route) => ({
41+
methods: route.methods,
42+
pattern: route.pattern,
43+
matcher: patternToRegex(route.pattern),
44+
}))
45+
}
46+
47+
function patternToRegex(pattern: string): RegExp {
48+
const source = pattern
49+
.split('/')
50+
.map((segment) => {
51+
if (segment.startsWith('*')) return '.*'
52+
if (segment.startsWith(':')) return '[^/]+'
53+
return segment.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
54+
})
55+
.join('/')
56+
57+
return new RegExp(`^${stripTrailingSlash(source)}$`)
58+
}
59+
60+
function stripTrailingSlash(value: string): string {
61+
return value.replace(/\/+$/, '') || '/'
62+
}
63+
64+
/** `${...}` becomes a param placeholder; the query string is irrelevant to routing. */
65+
function normalizeUrl(url: string): string {
66+
return stripTrailingSlash(url.replace(/\$\{[^}]*\}/g, ':param').replace(/\?.*$/, ''))
67+
}
68+
69+
function walk(dir: string, out: string[] = []): string[] {
70+
for (const entry of readdirSync(dir)) {
71+
const path = join(dir, entry)
72+
if (statSync(path).isDirectory()) walk(path, out)
73+
else if (/\.(vue|ts|js)$/.test(entry)) out.push(path)
74+
}
75+
return out
76+
}
77+
78+
const CALL_PATTERN = /\.(get|post|put|patch|delete)\(\s*[`'"](\/[^`'"]*)[`'"]/g
79+
const GOTO_PATTERN = /\.goto\(\s*[`'"](\/[^`'"]*)[`'"]/g
80+
const FORM_COMPONENT_PATTERN =
81+
/<Form\b[^>]*?\bmethod=["']([a-z]+)["'][^>]*?\baction=["'](\/[^"']*)["']/gs
82+
83+
function collectCalls(files: string[]): Call[] {
84+
const calls: Call[] = []
85+
86+
for (const file of files) {
87+
const source = readFileSync(file, 'utf8')
88+
const lines = source.split('\n')
89+
const lineOf = (index: number) => source.slice(0, index).split('\n').length
90+
91+
const push = (index: number, method: string, url: string, label?: string) => {
92+
const line = lineOf(index)
93+
calls.push({
94+
file,
95+
line,
96+
method: method.toUpperCase(),
97+
url,
98+
source: label ?? lines[line - 1]?.trim() ?? '',
99+
})
100+
}
101+
102+
for (const match of source.matchAll(CALL_PATTERN)) push(match.index!, match[1], match[2])
103+
for (const match of source.matchAll(GOTO_PATTERN)) push(match.index!, 'GET', match[1], 'goto()')
104+
for (const match of source.matchAll(FORM_COMPONENT_PATTERN)) {
105+
push(match.index!, match[1], match[2], '<Form>')
106+
}
107+
}
108+
109+
return calls
110+
}
111+
112+
const routes = loadRoutes()
113+
const files = [...walk(join(repoRoot, 'inertia')), ...walk(join(repoRoot, 'tests/e2e'))]
114+
const calls = collectCalls(files)
115+
116+
const problems: string[] = []
117+
118+
for (const call of calls) {
119+
const url = normalizeUrl(call.url)
120+
const pathMatches = routes.filter((route) => route.matcher.test(url))
121+
122+
if (pathMatches.length === 0) {
123+
problems.push(
124+
`${relative(repoRoot, call.file)}:${call.line} ${call.method} ${url}\n` +
125+
` no route matches this path ${call.source}`
126+
)
127+
continue
128+
}
129+
130+
if (!pathMatches.some((route) => route.methods.includes(call.method))) {
131+
const allowed = [...new Set(pathMatches.flatMap((route) => route.methods))].join(', ')
132+
problems.push(
133+
`${relative(repoRoot, call.file)}:${call.line} ${call.method} ${url}\n` +
134+
` route ${pathMatches[0].pattern} only accepts ${allowed} ${call.source}`
135+
)
136+
}
137+
}
138+
139+
console.log(`Route table: ${routes.length} routes`)
140+
console.log(`Frontend + e2e calls checked: ${calls.length}`)
141+
142+
if (problems.length > 0) {
143+
console.error(`\n${problems.length} call(s) cannot reach a route:\n`)
144+
for (const problem of problems) console.error(` ${problem}\n`)
145+
console.error('A verb mismatch answers 404, not 405 — fix the call or the route.')
146+
process.exit(1)
147+
}
148+
149+
console.log('All calls match a registered route.')

0 commit comments

Comments
 (0)