Close all thirteen app functions in the migration, not by hand
B4 was recorded as open against the deployment. It was not open there - the ten original functions had been closed by hand - but it was open everywhere else. A database restored from a backup of the deployment comes back with 10 of 13 PUBLIC-executable, because a hand-applied GRANT is a property of one database and does not survive a restore. Observed on scratch, built from the pre-0003 dump. So 0003 now revokes from PUBLIC on all thirteen, by full signature taken from pg_proc.oid::regprocedure: a bare () does not resolve for a function taking one argument, and REVOKE on a nonexistent name aborts the whole migration. Three findings while doing it, none of which would have shown up without running the tests: - My own three guard functions went in PUBLIC-executable, reproducing the exact defect B4 is about. - Revoking PUBLIC without regranting broke five tenancy tests with a permission denied error on the platform-admin helper. Every RLS policy calls it, so a missing grant here looks like a broken policy rather than a missing grant. Regranted to the roles that actually read and write, guarded by EXISTS so a role added later is covered by the next migration rather than by luck. - The revoke block originally sat before the functions were created, so it failed on a fresh restore and passed on the already-migrated database - passing by accident, the failure mode this project keeps hitting. Both revoke blocks now sit after the triggers, and the fresh-restore path is the one verified. Verified by restoring the pre-0003 backup into scratch, applying 0003, and running the suite: 0 of 13 PUBLIC-executable, 4 guards, 474 passed, 27 skipped. Re-applying is a no-op. Deployment data unchanged (users=2 orgs=3 audit=43 perms=20); health 200, heartbeat 401 without a credential and 200 with one, so the revoke broke nothing. G2 is still open: 26 tables have RLS enabled but not forced, and the application connects as the owner.
This commit is contained in:
umbra audio
@@ -776,8 +776,8 @@ that is a finding and you should report it as a blocker rather than proceeding.
|
||||
**Stage 1 needs no schema change and no new endpoint.** `node_heartbeats.raw` is an open
|
||||
`jsonb`, so the node report rides inside the heartbeat that already exists. Your report is
|
||||
already read-only, already secret-free, and already emits per-project schema fingerprints —
|
||||
it was built for exactly this. Send it under a `raw.node_report` key alongside the usual
|
||||
fields.
|
||||
it was built for exactly this. Send it as the **top-level `measurements` object** (§10.6 gives
|
||||
the exact shape, which is flat — there is no `raw` key and no `agent` object).
|
||||
|
||||
### 10.3 Why it starts read-only
|
||||
|
||||
@@ -844,26 +844,88 @@ second credential.
|
||||
|
||||
### 10.6 What your heartbeat should carry
|
||||
|
||||
`node_heartbeats.raw` is an open `jsonb`. Your existing
|
||||
`scripts/umbra_node_report.py --json` output rides inside the heartbeat as-is — no migration
|
||||
on this side, no new endpoint, no new schema on yours. Send:
|
||||
> **Corrected 2026-10-02, after this document was already in your hands.** The payload shape
|
||||
> originally given here was `{"agent": {"version": ...}, "raw": {...}}`. That shape is wrong.
|
||||
> It returns `200 OK` and stores nothing — verified, not assumed: the row came back with
|
||||
> `agent_version=NULL` and `raw={}`. An earlier version of this section was wrong in exactly
|
||||
> the way that is hardest to notice, so read the correction rather than trusting the shape you
|
||||
> were first given. If you have already been sending the old shape, stop; nothing you have sent
|
||||
> has been recorded.
|
||||
|
||||
The request body is **flat**. There is no `agent` object and no `raw` key:
|
||||
|
||||
```json
|
||||
{"agent": {"version": "<your version>"}, "raw": { ...your existing report... }}
|
||||
{
|
||||
"agent_version": "umbra-audio-0.1.0",
|
||||
"active_jobs": 0,
|
||||
"resident_models": [],
|
||||
"devices": [],
|
||||
"measurements": { ...your existing report... }
|
||||
}
|
||||
```
|
||||
|
||||
Two things the response tells you, both earned rather than declared:
|
||||
Everything except `measurements` is optional and defaults to empty. **`measurements` is an open
|
||||
`jsonb` and is written verbatim into the `raw` column**, so your existing
|
||||
`scripts/umbra_node_report.py --json` output rides inside it as-is — no migration on this side,
|
||||
no new endpoint, no new schema on yours.
|
||||
|
||||
Keys that have a name of their own are lifted out into real columns; everything else stays in
|
||||
`raw` and is queryable as JSON. Recognised: `cpu_busy_pct`, `ram_free_mb`, `vram_free_mb`,
|
||||
`vram_total_mb`, `disk_free_mb`, `load_avg_1m`.
|
||||
|
||||
The working command:
|
||||
|
||||
```sh
|
||||
curl -sS -X POST https://umbratrans.bdmurphy.me/api/node/heartbeat \
|
||||
-H 'Authorization: Bearer <KEY>' \
|
||||
-H 'Content-Type: application/json' \
|
||||
-d '{
|
||||
"agent_version": "umbra-audio-0.1.0",
|
||||
"active_jobs": 0,
|
||||
"resident_models": [],
|
||||
"measurements": {
|
||||
"cpu_busy_pct": 0,
|
||||
"ram_free_mb": 0,
|
||||
"disk_free_mb": 0,
|
||||
"sql_tables": {"books": 4, "tracks": 11, "speakers": 461},
|
||||
"speaker_name_agreements": {"last_dwarf": [93, 68, 71], "dealhunter": [61, 64, 97]}
|
||||
}
|
||||
}'
|
||||
```
|
||||
|
||||
Arbitrary nested JSON survives intact. The example above stores
|
||||
`raw->speaker_name_agreements` as `{"dealhunter": [61, 64, 97], "last_dwarf": [93, 68, 71]}` —
|
||||
checked, not assumed.
|
||||
|
||||
**Transport note.** Use the hostname and no `Host` header is needed. If you reach the
|
||||
controller by IP address instead, you must send `Host: umbratrans.bdmurphy.me` or every
|
||||
request returns `400` — including `/api/health`, which makes a healthy service look dead.
|
||||
|
||||
### 10.7 What the response tells you
|
||||
|
||||
```json
|
||||
{"node_id":"...","name":"umbraaudio-m4","status":"online","schedulable":false,
|
||||
"status_reason":"online: no devices reported yet","devices":[],"heartbeat_interval_seconds":10}
|
||||
```
|
||||
|
||||
- **`status`** is `provisioning` until your first heartbeat, `online` after. It is never set
|
||||
by configuration.
|
||||
- **`schedulable`** is separate from `status` and will read `false` until you report at least
|
||||
one device. That is the honest answer: a node with no devices cannot be given work, and it
|
||||
is better that it says so than that it looks available.
|
||||
- **`status`** is `provisioning` until your first heartbeat, `online` after. Never set by
|
||||
configuration. A node an administrator has set to `draining` or `retired` is *not* promoted
|
||||
back by heartbeating.
|
||||
- **`schedulable`** is separate from `status` and reads `false` until you report at least one
|
||||
device that is not busy. That is the honest answer: a node with no devices cannot be given
|
||||
work, and it is better that it says so than that it looks available.
|
||||
- **`heartbeat_interval_seconds`** is `10`. Miss that window and the node goes `offline`.
|
||||
|
||||
To become schedulable, report a device. `kind` must be exactly one of `cpu`, `gpu_discrete`,
|
||||
`gpu_integrated`, `npu`, `tpu` — any other value is a `422`. `throughput_factor` is measured by
|
||||
the node rather than declared by a human: a declared factor is a guess that goes stale the
|
||||
moment quantisation or context length changes, and a stale factor sends work to the wrong
|
||||
machine.
|
||||
|
||||
```json
|
||||
"devices": [{"kind":"cpu","label":"M4","ram_total_mb":16384,"ram_free_mb":8192,
|
||||
"throughput_factor":1.0,"supports_accelerators":[]}]
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -581,6 +581,33 @@ ON CONFLICT (key) DO NOTHING;
|
||||
-- they should not have to do while holding an incident.
|
||||
--
|
||||
-- DELETE and UPDATE only. INSERT is how these tables are populated.
|
||||
-- EXECUTE is revoked from PUBLIC on all three, matching every other function in
|
||||
-- `app` (the ten that predate this migration were closed by hand; nothing in a
|
||||
-- migration did it, which is why it needs doing here explicitly).
|
||||
--
|
||||
-- A trigger function runs as SECURITY INVOKER, so the guard is only reached if the
|
||||
-- *calling* role may execute it. Revoking PUBLIC without granting the roles that
|
||||
-- actually write would therefore turn the guard into an outage rather than a
|
||||
-- protection - and an outage at least announces itself, whereas a guard that never
|
||||
-- fires does not. So the grants follow the revoke, to every role that writes these
|
||||
-- tables, and are re-issued on every apply so a role added later is covered by the
|
||||
-- next migration rather than by luck.
|
||||
--
|
||||
-- Verified after applying: 13 of 13 `app` functions non-PUBLIC-executable, and a
|
||||
-- non-owner role can still insert into `artifacts` and be refused on update.
|
||||
--
|
||||
-- The ten functions that predate this migration were closed by hand against the
|
||||
-- deployment, and nothing in any migration does it. That is why a database restored
|
||||
-- from a backup of the deployment comes back with all ten PUBLIC-executable again -
|
||||
-- observed, on the scratch database built from the pre-0003 dump, where 10 of 13 were
|
||||
-- open while the deployment itself had 3. A hand-applied grant is not a property of
|
||||
-- the schema, it is a property of one database, and it does not survive a restore.
|
||||
--
|
||||
-- So this closes all thirteen, not just the three added above. B4 was recorded as
|
||||
-- open against the deployment, where it was already fixed; the defect was never only
|
||||
-- about the deployment.
|
||||
--
|
||||
|
||||
--
|
||||
-- Note what this does *not* do: it does not stop the owner role from bypassing RLS
|
||||
-- (G2), and it does not stop a superuser. It stops an ordinary mistake from being
|
||||
@@ -641,6 +668,76 @@ BEGIN
|
||||
END
|
||||
$$;
|
||||
|
||||
DO $$
|
||||
DECLARE
|
||||
target text;
|
||||
BEGIN
|
||||
EXECUTE 'REVOKE ALL ON FUNCTION app.prevent_artifact_mutation() FROM PUBLIC';
|
||||
EXECUTE 'REVOKE ALL ON FUNCTION app.prevent_job_artifact_mutation() FROM PUBLIC';
|
||||
EXECUTE 'REVOKE ALL ON FUNCTION app.prevent_job_event_mutation() FROM PUBLIC';
|
||||
--
|
||||
IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'umbracontrol_api') THEN
|
||||
EXECUTE 'GRANT EXECUTE ON FUNCTION app.prevent_artifact_mutation() TO umbracontrol_api';
|
||||
EXECUTE 'GRANT EXECUTE ON FUNCTION app.prevent_job_artifact_mutation() TO umbracontrol_api';
|
||||
EXECUTE 'GRANT EXECUTE ON FUNCTION app.prevent_job_event_mutation() TO umbracontrol_api';
|
||||
END IF;
|
||||
IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'umbracontrol_worker') THEN
|
||||
EXECUTE 'GRANT EXECUTE ON FUNCTION app.prevent_artifact_mutation() TO umbracontrol_worker';
|
||||
EXECUTE 'GRANT EXECUTE ON FUNCTION app.prevent_job_artifact_mutation() TO umbracontrol_worker';
|
||||
EXECUTE 'GRANT EXECUTE ON FUNCTION app.prevent_job_event_mutation() TO umbracontrol_worker';
|
||||
END IF;
|
||||
|
||||
-- The read-only roles need EXECUTE on the policy helpers, not only on the guards.
|
||||
-- Every RLS policy calls app.is_platform_admin() or app.is_organization_member(),
|
||||
-- so revoking PUBLIC without this turns every SELECT into
|
||||
-- "permission denied for function is_platform_admin" - which looks like a
|
||||
-- permissions bug in the policy rather than a missing grant here. Found by
|
||||
-- running the tenancy tests as a non-owner role immediately after the revoke:
|
||||
-- five of them failed that way.
|
||||
FOREACH target IN ARRAY ARRAY[
|
||||
'app.can_access_project(uuid)',
|
||||
'app.can_view_user(uuid)',
|
||||
'app.current_user_id()',
|
||||
'app.is_organization_admin(uuid)',
|
||||
'app.is_organization_admin_for_project(uuid)',
|
||||
'app.is_organization_member(uuid)',
|
||||
'app.is_platform_admin()'
|
||||
] LOOP
|
||||
IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'umbracontrol_api') THEN
|
||||
EXECUTE format('GRANT EXECUTE ON FUNCTION %s TO umbracontrol_api', target);
|
||||
END IF;
|
||||
IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'umbracontrol_readonly') THEN
|
||||
EXECUTE format('GRANT EXECUTE ON FUNCTION %s TO umbracontrol_readonly', target);
|
||||
END IF;
|
||||
-- rls_probe is the non-owner role the tenancy tests use. It is not a
|
||||
-- deployment role; it exists so a policy can be observed refusing something,
|
||||
-- and it needs the helpers for exactly that.
|
||||
IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'rls_probe') THEN
|
||||
EXECUTE format('GRANT EXECUTE ON FUNCTION %s TO rls_probe', target);
|
||||
END IF;
|
||||
END LOOP;
|
||||
-- All thirteen, for the reason given above.
|
||||
FOREACH target IN ARRAY ARRAY[
|
||||
'app.can_access_project(uuid)',
|
||||
'app.can_view_user(uuid)',
|
||||
'app.current_user_id()',
|
||||
'app.is_organization_admin(uuid)',
|
||||
'app.is_organization_admin_for_project(uuid)',
|
||||
'app.is_organization_member(uuid)',
|
||||
'app.is_platform_admin()',
|
||||
'app.prevent_artifact_mutation()',
|
||||
'app.prevent_audit_mutation()',
|
||||
'app.prevent_job_artifact_mutation()',
|
||||
'app.prevent_job_event_mutation()',
|
||||
'app.set_updated_at()',
|
||||
'app.validate_user_role_scope()'
|
||||
] LOOP
|
||||
EXECUTE format('REVOKE ALL ON FUNCTION %s FROM PUBLIC', target);
|
||||
END LOOP;
|
||||
|
||||
END
|
||||
$$;
|
||||
|
||||
-- ---------------------------------------------------------------------
|
||||
-- 4. Row-level security
|
||||
-- ---------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user