feat(function-resolution): port the route-validated invocation surface - #125
feat(function-resolution): port the route-validated invocation surface#125pyramation wants to merge 1 commit into
Conversation
…e from constructive-db Adds function_resolution.create_invocation (the one request-role insert surface for a sync invocation, with route-validated anonymous access) and its grants, and teaches install_route_bindings to install the typed routes.anonymous column.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review complete. 🟠 2 high 💬 Inline comments (2)
This PR adds two new stored procedures to the function-resolution package:
Two high-severity findings: the route Reviewed commit: a873f4d |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds create_invocation and install_route_bindings stored procedures to the function-resolution package, but two high-severity issues (immutable anonymous route flag and access_channels type mismatch) can leave routes publicly reachable and break sync invocations.
Key findings
- 🟠 Route
anonymousflag is immutable, fail-open on revocation — install_route_bindings.sql:436 - 🟠
access_channelsqueried as array but stored as jsonb — create_invocation.sql:234
| @@ -447,7 +454,7 @@ BEGIN | |||
| ); | |||
|
|
|||
| EXECUTE query USING domain_id, entry_path, target_id, key_value, | |||
| install_route_bindings.site_id; | |||
| install_route_bindings.site_id, entry_anonymous; | |||
There was a problem hiding this comment.
🟠 security · high
Route anonymous flag is immutable, fail-open on revocation
install_route_bindings writes the route's anonymous column only in an INSERT guarded by WHERE NOT EXISTS on (domain_id, path) (install_route_bindings.sql:436); its only UPDATE (lines 475-489) backfills serving_site_field and never touches anonymous. Re-running with a document that removes anonymous: true to close a URL leaves the existing row anonymous = true, and create_invocation (which enforces r.anonymous for anonymous callers at line 287) keeps authorizing it, so the operator's revocation silently does not take effect and the URL stays publicly reachable. The same immutability also means a route newly declared anonymous never opens.
📋 Prompt for AI Agents
In packages/function-resolution/deploy/schemas/function_resolution/procedures/install_route_bindings.sql around lines 434-464, the anonymous flag is only written on INSERT and the WHERE NOT EXISTS guard (lines 438-440) skips existing routes, so a route once installed anonymous=true can never be revoked by re-running the install. Add an UPDATE, after the insert, that reconciles anonymous on already-present rows for the same (domain_id, path) and ownership key, mirroring the existing serving_site_field backfill at lines 475-489: UPDATE <routes_schema>.<routes_table> AS r SET anonymous = $6 WHERE r.domain_id = $1 AND r.path = $2 AND r.anonymous IS DISTINCT FROM $6 AND <routes_key pin>. This ensures removing anonymous: true from the document actually closes the route, preventing the fail-open case where a URL the operator closed remains anonymously reachable via create_invocation.
| 'SELECT ''sync'' = ANY(d.access_channels), d.anonymous_callable FROM %I.%I AS d WHERE d.id = $1', | ||
| definitions_schema, | ||
| definitions_table | ||
| ); | ||
|
|
||
| -- pgsql-lint-disable-next-line no-dynamic-sql -- lookup-only: the definitions plane is named by its own function_module registration | ||
| EXECUTE query INTO sync_callable, anonymous_callable USING v_definition_id; |
There was a problem hiding this comment.
🟠 bug · high
access_channels queried as array but stored as jsonb
In create_invocation.sql:234 the sync-eligibility check runs 'sync' = ANY(d.access_channels), which requires access_channels to be a text[] array column. The established consumer resolve_capabilities.sql:114 reads the same field as JSONB (jsonb_array_elements_text(v_definition->'access_channels')) and writes it back as JSONB at line 267. If the generated definitions plane stores access_channels as jsonb, ANY() over a non-array value raises a runtime error, so the new function fails on every sync invocation.
📋 Prompt for AI Agents
In packages/function-resolution/deploy/schemas/function_resolution/procedures/create_invocation.sql around lines 233-240, the dynamic query SELECT 'sync' = ANY(d.access_channels), d.anonymous_callable FROM %I.%I AS d WHERE d.id = $1 assumes access_channels is a text[] array column, but the same field is stored and read as jsonb by the existing consumer resolve_capabilities.sql:114 (jsonb_array_elements_text(v_definition->'access_channels')). Change the sync-channel check to read the column as jsonb, e.g. SELECT EXISTS (SELECT 1 FROM jsonb_array_elements_text(d.access_channels) AS c WHERE c = 'sync'), d.anonymous_callable FROM %I.%I AS d WHERE d.id = $1, so it matches the actual definitions-plane column type and the existing consumer's contract.
1 participant
Summary
@pgpm/[email protected]on npm predates the typed anonymous contract that landed in constructive-db (constructive-io/constructive-db#3595, constructive-io/constructive-db#3602), so the module that owns the sync ledger's insert surface is the one piece of that contract nobody can consume. This ports the drifted SQL from the constructive-db mirror tree — deploy/revert/verify triples plus the two plan entries — and repackagessql/.Two changes:
function_resolution.create_invocation(new, plus its grants change): the single request-role insert surface for a sync invocation. The caller's own role writes the row — no RLS bypass — and the anonymous arm authorizes itself by re-reading the matched route on the tenant's routing plane, so anonymous execution requires both keys: the definition consents (anonymous_callable) and the route exposes (routes.anonymous).access_channelsis transport eligibility and is deliberately not the signal. The scope-key column is keyed from the module's ownentity_field, withentity_id/entity_typeleft as attribution.install_route_bindings: an entry may now declareanonymous, installing the typed column instead of aconfig->>'anonymous'jsonb convention. Defaults to false, so a document that says nothing installs closed routes.Version stays 0.44.0 here — the label moves in the release commit (
lerna version+pgpm sync-versions), same as the previous sync (ef58619).Verified:
pnpm testin the package (83 passed, 8 suites) andpgpm test-packages --full-cycle(all modules passed, so deploy → verify → revert parity holds for the new triples).sql/pgpm-function-resolution--0.44.0.{sql,bundle.tar.gz}regenerated withpgpm packageand byte-identical to the constructive-db artifact.Link to Devin session: https://app.devin.ai/sessions/f288ec33e25c48bbb3b055dc2376c3aa
Open in Devin Desktop: https://app.devin.ai/desktop/session/f288ec33e25c48bbb3b055dc2376c3aa?variant=devin
Requested by: @pyramation