mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-15 08:02:53 +00:00
Diego ImbertandClaude Opus 5
e400daabb8
Five findings across three rounds were all the same choice. A set of live Postgres credentials
was living in `global_settings`, which has generic read, list, write, config-export and CLI
round-trip paths that know nothing about what they carry: the passwords reached the instance
config and its YAML editor, a full-row upsert of a neighbouring key erased the catalog,
`GET /settings/global/{key}` and the settings listing returned them raw, and this round the
redaction that fixed the last two turned `wmill instance push` into something that wipes every
password — a fix breaking the assumption the previous fix made. `POST /settings/global/datatable_roles`
could also empty it outside the lock.
The approved plan offered a table or `global_settings`, so this is the other option it already
allowed rather than a new design. `datatable_role` is a table: no generic settings path can read
it, list it, export it, write it or round-trip it, so none of the five needs a guard. The
redaction, the hidden/protected/agent-denylist entries and the JSON document all go with it.
One row per role also removes the read-modify-write the concurrency work was about: two
concurrent creates are two inserts, and the unique index on `name` is what settles a collision.
The advisory lock stays for the one window rows do not cover — `CREATE ROLE` is invisible to
another transaction until commit, so without it both creates pass their `pg_roles` check.
Also from this round: rename mappings are checked against the configuration they claim to
describe, since fork pointers are rewritten from them — a caller could otherwise submit
`main -> missing` against an unchanged config and repoint every fork of `main` at a name nothing
has, and `A -> B` plus `B -> C` moved what pointed at `A` all the way to `C`. And the warning
naming forks a delete stranded reached the response but not the screen: both the data table
settings save and the workspace delete now show it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR
22 lines
1.1 KiB
SQL
22 lines
1.1 KiB
SQL
-- The instance's data table role catalog: one row per Postgres login Windmill created for data
|
|
-- table access.
|
|
--
|
|
-- A table rather than a `global_settings` key, because the value is a set of live cluster
|
|
-- credentials and that table has generic read, list, write and CLI round-trip paths that know
|
|
-- nothing about what they are carrying. Every one of them is a way to leak the passwords or to
|
|
-- overwrite the catalog with a copy that has none, and a row nothing generic touches has none of
|
|
-- those. One row per role also makes two concurrent creates two inserts rather than a
|
|
-- read-modify-write over one document.
|
|
CREATE TABLE datatable_role (
|
|
id VARCHAR(50) PRIMARY KEY,
|
|
-- The Postgres role name, verbatim. Unique because it is the cluster's own key.
|
|
name VARCHAR(63) NOT NULL UNIQUE,
|
|
enabled BOOLEAN NOT NULL DEFAULT true,
|
|
-- Generated by Windmill, never entered by anyone, and never leaves the server.
|
|
pwd TEXT,
|
|
created_at TIMESTAMPTZ NOT NULL DEFAULT now()
|
|
);
|
|
|
|
GRANT ALL ON datatable_role TO windmill_user;
|
|
GRANT ALL ON datatable_role TO windmill_admin;
|