Skip to content

fix(webhooks): gate schedule/webhook creation on a live owning agent (#1445)#1453

Merged
vybe merged 7 commits into
devfrom
AndriiPasternak31/issue-1445
Jul 6, 2026
Merged

fix(webhooks): gate schedule/webhook creation on a live owning agent (#1445)#1453
vybe merged 7 commits into
devfrom
AndriiPasternak31/issue-1445

Conversation

@AndriiPasternak31

@AndriiPasternak31 AndriiPasternak31 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #1445

Root cause

POST /api/webhooks/{token} intermittently 404'd for a freshly-generated, valid token under concurrent load. The trigger is orphan schedules: an admin's can_user_access_agent returns True unconditionally with no existence check, so a schedule (and a real webhook token) could be minted on an agent that has no live agent_ownership row. That token then 404s deterministically at get_schedule_by_webhook_token, which since #1423 INNER-JOINs agent_ownership and filters deleted_at IS NULL. The full-suite flakiness was the module fixture creating schedules on the name-only test_agent_name (no ownership row) — an orphan whose token 404s.

Fix — no-orphan creation gate

Schedule and webhook creation now require a live owning agent:

  • Router (create_schedule, generate_webhook): access-check first, then 404 on not db.is_agent_live(name). Ordering matters — a low-priv caller gets a uniform 403 whether or not the agent exists (no 404-vs-403 enumeration oracle); only an admin/owner ever reaches the 404. Placed before the bug/design: schedule vs agent timeout precedence is silent + SIGKILL error message is ambiguous #929 timeout check so the gate can't be probed via that path.
  • DB chokepoint (db/schedules.py:create_schedule → None): re-enforced so the invariant holds for every caller (MCP, system-manifest deploy, future/internal), not just the router.
  • New predicate db.is_agent_live(name): checks agent_ownership.deleted_at IS NULL without joining users, matching the token-lookup predicate exactly. get_agent_owner INNER-JOINs users, so it would false-negative a live agent whose owner-user row is missing (FKs are off platform-wide) — a comment + a dedicated test pin why the fix must not reuse it.
  • Webhook 404 branches now emit distinct static log lines (webhook 404: malformed token / token lookup miss) so the next occurrence is grep-attributable in Vector. The raw token is never interpolated (log-injection / partial-secret leak on un-validated bytes).

get_schedule_by_webhook_token (#1423) is unchanged — the gate keeps it from ever seeing an orphan.

Tests

  • tests/test_webhook_triggers.py — hardened off orphan agents: new module-scoped webhook_agent fixture (real POST /api/agents, live ownership row); new TestWebhookCreationGate (missing-agent 404, soft-deleted-agent webhook 404).
  • tests/unit/test_1445_schedule_creation_gate.py — backend-agnostic (db_harness, SQLite + optional Postgres): is_agent_live truth table incl. the missing-owner-row invariant, and the create_schedule chokepoint (no-orphan for non-router callers + positive control).
  • tests/unit/test_929_timeout_validation.py — access-first ordering pin (no enumeration oracle) + no-orphan 404; existing timeout tests updated to stub the new gate.

Docs

architecture.md (Creation gate block), requirements/public-access.md, feature-flows/webhook-triggers.md + index row, docs/user-docs/api-reference/webhook-triggers.md.

Validation

/validate-pr clean — no secrets/config/build-packaging gaps; all four mandated doc surfaces updated; named regression tests present. One doc-drift typo caught and fixed in-branch (#1433→#1423 in the flow index). Full integration suite (requires a running stack) to be confirmed on the reviewer's env.

🤖 Generated with Claude Code

ap-31 and others added 6 commits July 4, 2026 19:52
@ap-31 @claude @AndriiPasternak31
fix(webhooks): gate schedule/webhook creation on a live agent (#1445)
2fbc9fd 
Root cause: PR #1433 gave get_schedule_by_webhook_token an INNER JOIN on
agent_ownership, so a schedule whose agent has no live ownership row now
404s at token lookup. can_user_access_agent returns True unconditionally
for admins with no existence check, so admin callers could mint orphan
schedules (with real webhook tokens) on never-created agents — tokens that
then 404 deterministically. This is the source of the "intermittent under
load" webhook 404s: a test-fixture artifact, not concurrency.

Fix at the creation boundary, fail-loud, keeping #1433's INNER JOIN intact:

- New db.is_agent_live(name) predicate: live agent_ownership row,
  deleted_at IS NULL, no users join — matches the token-lookup predicate
  exactly (get_agent_owner joins users → false-negative on a live agent
  with a missing owner row).
- routers/schedules.create_schedule: access-check FIRST (uniform 403, no
  404-vs-403 enumeration oracle), then is_agent_live 404, before the #929
  timeout check.
- routers/schedules.generate_webhook: is_agent_live 404 after the access
  check (defense-in-depth, not a pre-auth probe).
- db/schedules.create_schedule: chokepoint guard (is_agent_live → None →
  403) so the no-orphan invariant holds for every caller.
- webhooks.py: static, neutral per-branch 404 log lines (no token
  interpolation) so the next occurrence is grep-attributable.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ap-31 @claude @AndriiPasternak31
test(webhooks): harden webhook tests off orphan agents; gate regressi…
dd31c93 
…on (#1445)

- Move the WHOLE module off the name-only `test_agent_name` fixture (which
  creates no ownership row) onto a module-scoped `webhook_agent` fixture
  that POSTs a real agent (live ownership row, no flaky running-wait,
  mirrors the #1423 throwaway-agent pattern). Post-gate, `_create_test_schedule`
  asserts 201, so every test — generation AND trigger — needs a real agent.
- New TestWebhookCreationGate: creating a schedule on a never-created agent
  now 404s (was 201 → orphan); generate_webhook on a soft-deleted agent 404s.
- Fix the direct route-handler unit tests (test_929) to stub the new
  db.can_user_access_agent / db.is_agent_live gate so they reach the
  timeout-enforcement path under test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ap-31 @claude @AndriiPasternak31
docs(webhooks): document the live-agent creation gate (#1445)
96420cd 
- architecture.md WEBHOOK-001: creation gate note (is_agent_live, 404/403
  ordering, chokepoint) — idempotency sentence untouched (#1437's territory)
- requirements/public-access.md §15.1g: creation-gated-on-live-agent bullet
- feature-flows/webhook-triggers.md: management-endpoint auth + creation-gate
  paragraph; failure-mode table notes (malformed/lookup-miss log lines,
  orphan schedules no longer creatable)
- feature-flows.md index: Recent Updates row
- user-docs/api-reference/webhook-triggers.md: add the missing WEBHOOK-001
  schedule-webhook endpoints + the public creation precondition

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ap-31 @claude @AndriiPasternak31
test(webhooks): unit-pin the #1445 access-first ordering + no-orphan 404
60fe0d2 
Two importlib route-handler tests (no live stack):
- access-check-first => uniform 403 with is_agent_live never consulted
  (enumeration-oracle guard, plan Decision #5)
- authorised caller on a dead agent => 404, db.create_schedule never reached

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ap-31 @claude @AndriiPasternak31
docs(webhooks): sync flow doc — is_agent_live gate + 404-branch logs (#…
b1e4068 
…1445)

/sync-feature-flows pass: DB Methods table gains the is_agent_live gate
predicate and spells out the #1423 soft-delete-aware token-lookup INNER JOIN;
the public-trigger steps note the static per-branch 404 log lines.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ap-31 @claude
test(webhooks): DB-layer #1445 gate unit tests; fix flow-index issue ref
7ac8310 
Add tests/unit/test_1445_schedule_creation_gate.py — the backend-agnostic
(db_harness) coverage the router/integration tests can't reach:
is_agent_live (no users-join, matches the token-lookup predicate; live agent
with a missing owner-user row still reads live) and the create_schedule
chokepoint (no-orphan for MCP / system-manifest-deploy / any non-router caller).

Fix the feature-flows index row to cite #1423 (the soft-delete token-lookup
issue) instead of #1433 (its PR) — matches the code docstring, architecture.md,
requirements, and the flow doc.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@AndriiPasternak31
AndriiPasternak31 marked this pull request as ready for review July 4, 2026 21:42
@AndriiPasternak31
AndriiPasternak31 requested review from dolho and vybe July 4, 2026 21:43
@AndriiPasternak31 AndriiPasternak31 self-assigned this Jul 4, 2026
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

@claude
Merge origin/dev into issue-1445 — reconcile #186 uniform-404 with the …
e550e31 
…#1445 live-agent gate in generate_webhook

Keep dev's removal of the inline can_user_access_agent 403 (AuthorizedAgent
now applies the uniform-404 access gate, #186) and keep the #1445
is_agent_live defense-in-depth check after it. Verified: 48 unit tests pass
(test_1445_schedule_creation_gate, test_929_timeout_validation,
test_186_enumeration_uniformity).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vybe
vybe approved these changes Jul 6, 2026
View reviewed changes

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated via /validate-pr: conventional commits, Fixes #1445 (P1 type-bug, theme-reliability), root-cause verified (orphan fixture, deterministic — not a WAL race), defense-in-depth at router + DB chokepoint, no revert of #1433/#1436, named regression tests. Dev-merge code conflict in generate_webhook resolved: kept #186's uniform-404 AuthorizedAgent gate (inline 403 stays removed) + kept the #1445 is_agent_live check; 48 affected unit tests pass locally on the resolved head.

@vybe
vybe enabled auto-merge (squash) July 6, 2026 09:00
@vybe
vybe merged commit b56d688 into dev Jul 6, 2026
20 checks passed
@vybe vybe mentioned this pull request Jul 6, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vybe vybe vybe approved these changes
@dolho dolho Awaiting requested review from dolho

Assignees

@AndriiPasternak31 AndriiPasternak31

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AndriiPasternak31 @vybe @ap-31