Repository navigation
Commit efcbac7
fix(trigger-api,spec): one answer for every inbound hook post that does not verify (#22822)
Fixes #22806
Clause-②: yes
Dispatched by the `domain:services` seat 1 PM (claim `6107768529`,
branch `claude/issue-22806-uniform-hook-refusal`). Direction: triage
`6107336164`. Disclosure: the card is `security`, so this body names
classes and positions only.
## What changes
`ApiTrigger.handleRequest`
(`packages/triggers/trigger-api/src/api-trigger.ts`) now gives every
inbound post that does not verify the same answer: one status and one
body. That covers an unknown flow, a wrong hook id, an absent signature
and a bad one, whatever hook id the flow is armed with. The answer is
`401 INVALID_SIGNATURE`, the refusal a bad signature already got.
- **FROM → TO.** An unknown flow and a wrong hook id used to answer `404
RESOURCE_NOT_FOUND`. They now answer the same `401 INVALID_SIGNATURE` as
an absent or bad signature.
- **The order is unchanged.** The hook is matched before its secret is
read. So the secret-unavailable `503`, the residual triage kept, still
answers only a post that names an armed flow and its hook id. A wrong
hook id never reaches the secret read, so it gets the uniform `401` even
while the secret is unreadable.
- **The cause is logged.** Each refusal logs one `warn` line with
structured fields `{ cause, flowName }`. `cause` is one of
`unknown-flow`, `wrong-hook-id`, or `signature-` followed by core
`verifyHttpSignature`'s own reason (`absent`, `malformed`, `mismatch`,
`outside-window`). The line never carries the secret, the signature
value or the body.
- **The "no oracle" comment is restated.** It now claims what ships, and
nothing more: the answer (status and body) is uniform. It names the
`503` as the one residual.
- **`packages/spec`, docblock only, under the cross-domain exception
triage named.** The `ITriggerApiService.handleInboundHook` docblock now
states the one refusal (FROM → TO) and names the `503` as its residual.
Types are unchanged. The spec's contract test
(`trigger-api-service.test.ts`) pins types only and no status text, so
it does not move.
- **Changeset.** `.changeset/22806-uniform-hook-refusal.md`, `minor` for
`@objectstack/trigger-api` and `@objectstack/spec`. It states FROM → TO,
what each kind of sender sees, the log fields and the `503` residual.
## Measurement at the door (step 1)
These are answers from the real `handleRequest`, armed through `start()`
as the engine arms it. They were read on `c11b75870` (the base) and on
this branch, unsigned and with a bad signature.
| post | base `c11b75870` | this branch |
|:--|:--|:--|
| a flow that is not armed | `404 RESOURCE_NOT_FOUND` | `401
INVALID_SIGNATURE` |
| an armed flow on the default hook id | `401 INVALID_SIGNATURE` | `401
INVALID_SIGNATURE` |
| an armed flow on a custom hook id, posted with another hook id | `404
RESOURCE_NOT_FOUND` | `401 INVALID_SIGNATURE` |
| an armed flow on a custom hook id, posted with that hook id | `401
INVALID_SIGNATURE` | `401 INVALID_SIGNATURE` |
| an armed flow whose secret cannot be read, its own hook id | `503
SERVICE_UNAVAILABLE` | `503 SERVICE_UNAVAILABLE` (the residual,
unchanged) |
| an armed flow whose secret cannot be read, another hook id | `404
RESOURCE_NOT_FOUND` | `401 INVALID_SIGNATURE` |
| CONTROL: a valid post, default and custom hook id | `202` | `202` |
On the base, the unsigned answer separated armed from unarmed names only
for a flow left on the default hook id. A custom hook id already closed
it, as long as that hook id stayed unknown. With this change, the answer
separates nothing in either case.
## Boundary flag: the status (for the contract-tier review)
The status is the reviewer's call, not this PR's. Both candidates are
already declared codes. This PR implements `401 INVALID_SIGNATURE` and
recommends it. If the review rules for `404`, that is a patch round.
| axis | A: uniform `401 INVALID_SIGNATURE` (implemented) | B: uniform
`404 RESOURCE_NOT_FOUND` |
|:--|:--|:--|
| Real business need | Correct sender: `202`, no change. A sender with a
wrong secret, wrong signing code, re-serialised body or skewed clock
still gets `401`, so it looks at its signature, which is right. A sender
with a wrong URL (a flow that is not armed, a rotated hook id) gets
`401` where it got `404`; the `warn` line names `unknown-flow` /
`wrong-hook-id`. Measured: no SDK client method builds a
`/automation/hooks/*` URL (`TRIGGER_API_ROUTE_LEDGER`), so no
first-party consumer reads the old `404`. | Correct sender: no change. A
sender with a bad signature, including a replay or a clock outside the
window, gets `404 No such hook`, which points it at the URL when the
signature is what is wrong. A sender with a wrong URL gets `404`, as
before. |
| Long-term soundness | One code means "not verified". The error-code
ledger row `@objectstack/trigger-api: INVALID_SIGNATURE` stays true with
no ledger edit. Pending `.changeset/22769-versioned-http-signature.md`
("the same `401 INVALID_SIGNATURE` a bad signature gets") stays true. |
`INVALID_SIGNATURE` loses its only emitter. The ledger row goes stale,
so the spec needs a code retirement or keeps a dead row, which goes
beyond a docblock edit. Pending `22769`'s refusal sentence becomes false
in the same release. |
| Keeping AI from writing it wrong | An agent wiring a sender that reads
`401` fixes the signature, the most common integration fault. A wrong
URL is the rarer case, and the log names it. | An agent that reads `404`
rewrites the URL while the secret is wrong: the misleading direction
lands on the most common fault. |
| No scope spread | Docblock-only spec edit, no new code, no ledger
change. | Adds an error-code ledger change to the spec diff. |
Recommendation: **A**. On every axis it serves the common failure
correctly and keeps the declared vocabulary true without touching the
ledger.
## Pins (all on the real `handleRequest`)
In `packages/triggers/trigger-api/src/api-trigger.test.ts`, the block
"one answer for every post that does not verify" covers 24 posts. Five
addresses (two unknown, three armed, including a custom hook id posted
with another hook id) are each sent unsigned, malformed, and badly
signed in both forms. Four more posts carry a right secret and still
must not verify: the wrong hook id, another flow's secret, and a
timestamp outside the window.
- Every answer equals every other. The first is `401` /
`INVALID_SIGNATURE` / `success: false`. Message prose is not pinned.
Nothing is enqueued.
- The log records each refusal's cause once, asserted by its `cause`
field, not prose. No logged argument contains either secret, the posted
signature value, or a body marker.
- CONTROL: a valid post is accepted under the default and a custom hook
id, in both signature forms.
- CONTROL: the `503` residual is unchanged, signed or not. A wrong hook
id on that flow gets the uniform `401` and never reads the secret.
- Six existing pins that asserted the old `404` now assert `401` /
`INVALID_SIGNATURE`: the unknown-flow-versus-wrong-hookId pin, the three
refused-to-arm pins, the `stop()` pin (now with a valid signature), and
the read-time-secret wrong-hook-id pin.
Red before green: on the base source, with the pins in place, 8 failed
and 19 passed (27).
### Ablations (committed state `f26b2f45f`, through
`scripts/ablation-replace.mjs`, which wraps the run and restores)
1. **The old distinct answer on the match branch.** The two match
refusals were replaced with one `404 RESOURCE_NOT_FOUND` return. The
anchor went from 1 hit to 0, and the blob went from `a920e032dcdc` to
`16afdd2829de`. Result: 8 failed, 19 passed (27), the same eight pins as
the base. Restored: blob equals HEAD (`a920e032dcdc`), and `git diff
HEAD` is empty.
2. **The logged cause dropped.** The `cause` field was removed from the
refusal line's fields (anchor 1 hit to 0, blob `a920e032dcdc` to
`a7bb3a4e349f`). Result: 1 failed, 26 passed (27), the log pin.
Restored: blob equals HEAD, and `git diff HEAD` is empty.
The pins import `./api-trigger.js` (source). `@objectstack/core` is
aliased to source in this package's `vitest.config.ts`, so no `dist/` is
on the path.
## Verification
All of the following ran at head `9f82771b0`, after the last commit.
Builds, tests and gates ran under `scripts/pm/os-verify-lock.sh`, and
each exit code was recorded to disk before any pipe.
- **① Dependency closure:** `pnpm --filter
'@objectstack/trigger-api^...' build`, exit 0. After the docblock
commit, spec, trigger-api and the packages the prerequisite gates read
were rebuilt (`turbo run build`, exit 0).
- **② Affected packages:**
- `pnpm --filter @objectstack/trigger-api test`: 2 files, 42 passed.
- `pnpm --filter @objectstack/trigger-api typecheck`: exit 0. Its
`tsconfig` includes `src/**/*`, which takes in the test file.
- `pnpm --filter @objectstack/spec test`: 648 files, 19342 passed, 1
todo.
- `pnpm --filter @objectstack/spec typecheck`: exit 0, at `f26b2f45f`.
The only later change is a comment.
- `pnpm --filter @objectstack/spec check:generated`: "All 14 generated
artifacts are up to date".
- **③/④ Derived gates:** `node scripts/pm/dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack` gave 86 commands, the same list
before and after the last commit. All 86 ran with exit 0. Reconciled
with `--ran` (each line carrying `:: exit N`): "86 derived famil(ies)
accounted for — 86 run, 0 NOT-MEASURED (a DERIVED zero — all 86 recorded
an exit code and none of them is 3)".
- **Also run, outside the derivation, all exit 0:**
`check:error-code-provenance` (spec), `check:error-code-casing`,
`check-changeset-fixed.mjs` and `check:route-ledger-census`.
- **Left to CI:** the path-scheduled CI jobs and type-check lanes that
`dispatch-gates` lists as outside its derivation, and the repo-wide
`pnpm lint`.
## Acceptance notes (noted, not filed)
- **What the claim covers.** The answer is uniform in status and body.
The code makes no claim about response time across the hook lookup, and
that was not measured. Carrier: none.
- **Pending changeset text.**
`.changeset/22772-spec-trigger-api-inbound-hook-member.md` (unreleased)
still says an unknown flow or wrong hook id answers `404`. If both ship
in one release, this PR's changeset states the FROM → TO beside it. That
changeset belongs to another card and is not edited here. Carrier:
whoever compiles that release's notes.
- **Signature-form text in the docblock.** The verification bullet in
`handleInboundHook`'s docblock still names only the body-only form,
while the runtime has also accepted the timestamped form since
`7b0a9c93c`. The accepted forms are outside this dispatch, so the merged
wording is kept as is. Carrier: none.
- **Log volume.** One `warn` line per refused post means anonymous
traffic can add lines at request rate. That is the price of keeping the
cause visible to the operator. No deduplication is added.
- **No `WWW-Authenticate` header.** The route has never sent one with
its `401`, and this change keeps that.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent e040b8c commit efcbac7
4 files changed
Lines changed: 265 additions & 31 deletions
File tree
- .changeset
- packages
- spec/src/contracts
- triggers/trigger-api/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
46 | | - | |
47 | | - | |
48 | | - | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
55 | 49 | | |
56 | 50 | | |
57 | 51 | | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
58 | 57 | | |
59 | 58 | | |
60 | | - | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
61 | 68 | | |
62 | 69 | | |
63 | 70 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
82 | 82 | | |
83 | 83 | | |
84 | 84 | | |
85 | | - | |
| 85 | + | |
86 | 86 | | |
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
90 | | - | |
| 90 | + | |
| 91 | + | |
91 | 92 | | |
92 | 93 | | |
93 | 94 | | |
| |||
115 | 116 | | |
116 | 117 | | |
117 | 118 | | |
118 | | - | |
119 | | - | |
| 119 | + | |
| 120 | + | |
120 | 121 | | |
121 | | - | |
122 | | - | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
123 | 125 | | |
124 | 126 | | |
125 | 127 | | |
| |||
154 | 156 | | |
155 | 157 | | |
156 | 158 | | |
157 | | - | |
158 | | - | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
159 | 165 | | |
160 | 166 | | |
161 | 167 | | |
| |||
213 | 219 | | |
214 | 220 | | |
215 | 221 | | |
216 | | - | |
| 222 | + | |
217 | 223 | | |
218 | 224 | | |
219 | 225 | | |
220 | | - | |
| 226 | + | |
| 227 | + | |
221 | 228 | | |
222 | 229 | | |
223 | 230 | | |
| |||
357 | 364 | | |
358 | 365 | | |
359 | 366 | | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
0 commit comments