Skip to content

feat(support): widget public-key registry and not-revoked resolution gate#35

Merged
hcho22 merged 5 commits into
mainfrom
feat/us-072-widget-keys
Jun 25, 2026
Merged

feat(support): widget public-key registry and not-revoked resolution gate#35
hcho22 merged 5 commits into
mainfrom
feat/us-072-widget-keys

Conversation

@hcho22

@hcho22 hcho22 commented Jun 25, 2026

Copy link
Copy Markdown
Owner

What Changed

  • Added the widget_keys table (supabase/migrations/20260624130000_widget_keys.sql): a non-secret per-workspace public_key registry with admin-only (role='admin') RLS on SELECT/INSERT/UPDATE, no DELETE policy (rotation is issue-new + revoke-old), and a BEFORE UPDATE trigger (_widget_keys_revoke_guard) that makes revoked_at a one-way DB-enforced latch (cannot be cleared or moved once set).
  • Added backend/widget_keys.py pure primitives (generate_public_keywk_pk_<urlsafe>, is_widget_public_key shape guard) and four FastAPI endpoints in backend/main.py: admin POST/GET /api/support/widget-keys and POST /api/support/widget-keys/{key_id}/revoke (under the admin's own JWT), plus anonymous POST /widget/keys/resolve reading under the service role and gating on revoked_at IS NULL. First issuance best-effort triggers lazy US-069 bot provisioning; all four endpoints map upstream PostgREST 5xx → 502 and 4xx/validation → 400.
  • Added backend/test_us072_widget_keys.py (unit primitives + integration/security layer driving the real app over HTTP) and updated docs (AGENTS.md, README.md, backend/.env.example, PRD task file) for the new widget-key surface.

Risk Assessment

✅ Low: Well-bounded, thoroughly tested feature whose security-sensitive parts (admin RLS, one-way revoke trigger, service-role-only public resolution that leaks no topology) are correct and pinned by tests, with both prior-round findings already addressed and remaining hardening explicitly deferred to scoped follow-up stories.

Testing

Ran the existing suite and a real HTTP API driver against live Supabase. All behaviors held and the DB and worktree are clean.

Pipeline

Updates from git push no-mistakes

⏭️ **intent** - skipped

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed (2) ✅
  • ⚠️ backend/main.py:2558 - Upstream-error mapping is applied to only one of the four new widget-key endpoints. issue_widget_key was deliberately hardened (commit 070a1df, "map upstream 5xx to 502") to translate PostgREST 401/403/5xx/4xx into clean HTTP statuses, but its three siblings still call a bare r.raise_for_status(): revoke_widget_key (line 2558), list_widget_keys (line 2535), and the public resolve path _resolve_widget_key (line 2455). As a result an upstream PostgREST 5xx surfaces as an unhandled 500 (not 502), and — most concretely — a malformed non-UUID key_id on POST /api/support/widget-keys/{key_id}/revoke (or a malformed workspace_id on list) makes PostgREST return a 4xx that propagates as a 500 instead of a clean 4xx. The 404-on-empty-rows logic in revoke is correct; only the unhandled upstream/validation errors are the gap. This matches the broader codebase convention (the US-071 widget endpoints also use bare raise_for_status), so impact is low, but it leaves the stated intent of the review commit half-applied. Wrap these three in the same httpx.HTTPStatusError mapping used by issue_widget_key.

🔧 Fix: map upstream errors on sibling widget-key endpoints to 502/400
1 info still open:

  • ℹ️ supabase/migrations/20260624130000_widget_keys.sql:44 - The migration states revocation is "a one-way flip to a timestamp (never un-revoked)", but unlike the US-067 conversations status machine (which pins its one-way latch with a BEFORE-UPDATE trigger, per the project's "enforced in the DB, not just the service layer" stance), nothing enforces this here. The widget_keys_update_admin policy (line 88) allows an admin to update any column on a key in their workspace, including setting revoked_at back to NULL via a direct PostgREST PATCH — re-activating a key the audit trail records as revoked. The revoke endpoint never does this, so the gap is only reachable by an admin calling PostgREST directly; impact is bounded since un-revoking grants no privilege an admin lacks (they can issue a fresh key anyway). Flagging the inconsistency between the documented one-way invariant and the absence of DB enforcement; if revocation is meant to be irreversible for audit integrity, a trigger rejecting non-null -> null transitions on revoked_at would pin it the way US-067 pins status.

🔧 Fix: enforce one-way widget-key revoke latch via DB trigger
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • python -m backend.test_us072_widget_keys passes
  • E2E HTTP driver against real FastAPI app exercises admin issue list revoke and public resolve
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

hcho22 added 5 commits June 25, 2026 08:33
…072)

Add the public widget key registry and the resolution gate that every key
passes before anything is minted/created (ADR-0008).

- Migration 20260624130000_widget_keys.sql: widget_keys table (non-secret
  public_key unique, allowed_origins for US-073, revoked_at NULL=active) with
  RLS admin-only SELECT/INSERT/UPDATE gated on workspace_membership.role='admin'
  - the one legitimate place role enters a predicate (key management IS the
  admin action). No DELETE policy (rotate = revoke, retained for audit).
- backend/widget_keys.py: pure primitives (generate_public_key -> wk_pk_<urlsafe>,
  is_widget_public_key shape guard).
- main.py: admin-authed /api/support/widget-keys (issue/list/revoke under the
  admin's own JWT so admin RLS is the authorization) + anonymous
  /widget/keys/resolve (service-role read gating on revoked_at IS NULL, leaking
  no workspace topology). _ensure_workspace_bot is the single, isolated,
  best-effort first-key bot-provisioning hook to US-069's documented contract;
  degrades to a logged deferral until US-069's primitive is present.
- test_us072_widget_keys.py: unit primitives + DB/security layer (revoked key
  mints nothing, admin-only reads/writes, live conversation survives revocation).

Resolution gates on not-revoked: a revoked/unknown key resolves to zero rows and
mints nothing, while the opaque per-conversation token (US-071) is independent of
the key once minted, so revoking never terminates a live conversation.
@vercel

vercel Bot commented Jun 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agentic-rag Ready Ready Preview, Comment Jun 25, 2026 5:56pm

@github-actions

Copy link
Copy Markdown
Contributor

Retrieval eval — PR vs main

n = 50 questions × 3 modes (vector, keyword, hybrid) on a 14-chunk corpus. PR ran in 158.02s; main in 81.5s.

Headline (each cell: PR value, Δ vs main)

Mode recall@5 MRR nDCG@5
vector 0.860 (±0.000) 0.772 (±0.000) 0.779 (±0.000)
keyword 0.110 (±0.000) 0.120 (±0.000) 0.112 (±0.000)
hybrid 0.860 (±0.000) 0.759 (±0.000) 0.769 (±0.000)

Per-category recall@5

Mode single_chunk multi_hop adversarial paraphrase
vector 0.900 (±0.000) 0.933 (±0.000) 0.600 (±0.000) 1.000 (±0.000)
keyword 0.250 (±0.000) 0.033 (±0.000) 0.000 (±0.000) 0.000 (±0.000)
hybrid 0.900 (±0.000) 0.933 (±0.000) 0.600 (±0.000) 1.000 (±0.000)

Comment is updated in place on each push by .github/workflows/retrieval-eval.yml (US-035). Comment-only — never blocks the build.

@hcho22 hcho22 merged commit 943b527 into main Jun 25, 2026
3 checks passed
@hcho22 hcho22 deleted the feat/us-072-widget-keys branch June 25, 2026 18:06
hcho22 added a commit that referenced this pull request Jun 25, 2026
US-072 (widget_keys registry + not-revoked resolution gate) merged via PR #35
(squash commit 943b527). Point the US-072 status block + acceptance-criteria
notes and the Epic E row at the merged PR/commit instead of the gate's
pre-squash fix SHAs (which the squash merge discarded), and record the
DB-enforced one-way revoke latch and live first-key bot provisioning as shipped.
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