mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 16:02:36 +00:00
Diego ImbertandClaude Opus 5
1db77dda87
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
46 lines
2.1 KiB
SQL
46 lines
2.1 KiB
SQL
-- A data table under roles in `test-workspace`, and a fork whose entry points at it rather than
|
|
-- carrying a copy. `test-user-2` is a non-admin of the parent and an admin of the fork: the shape
|
|
-- the pointer exists for.
|
|
|
|
-- Empty registry: role provisioning grants CONNECT on every database named here, and the data
|
|
-- table's `dt_main` is a name in workspace settings, not a database that exists.
|
|
INSERT INTO global_settings (name, value) VALUES
|
|
('custom_instance_pg_databases', '{"user_pwd": "pw", "databases": {}}'::jsonb)
|
|
ON CONFLICT (name) DO UPDATE SET value = EXCLUDED.value;
|
|
|
|
INSERT INTO datatable_role (id, name, enabled, pwd) VALUES ('role1', 'analytics', true, 'pw');
|
|
|
|
UPDATE workspace_settings SET datatable = '{
|
|
"datatables": {
|
|
"main": {
|
|
"database": {"resource_type": "instance", "resource_path": "dt_main"},
|
|
"permissions": {
|
|
"default_role": "role1",
|
|
"roles": {
|
|
"admin": {"tenants": []},
|
|
"role1": {"tenants": ["u/test-user-2", "g/analysts", "f/finance"]}
|
|
}
|
|
}
|
|
}
|
|
}
|
|
}'::jsonb WHERE workspace_id = 'test-workspace';
|
|
|
|
INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES
|
|
('test-workspace', 'analysts', 'Analysts', '{}');
|
|
INSERT INTO folder (workspace_id, name, display_name, owners, extra_perms) VALUES
|
|
('test-workspace', 'finance', 'finance', '{}', '{}');
|
|
|
|
INSERT INTO workspace (id, name, owner, parent_workspace_id) VALUES
|
|
('wm-fork-dt', 'fork of test-workspace', 'test2@windmill.dev', 'test-workspace');
|
|
INSERT INTO workspace_key (workspace_id, kind, key) VALUES ('wm-fork-dt', 'cloud', 'test-key');
|
|
INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES
|
|
('wm-fork-dt', 'all', 'All users', '{}');
|
|
INSERT INTO usr (workspace_id, email, username, is_admin, role) VALUES
|
|
('wm-fork-dt', 'test2@windmill.dev', 'test-user-2', true, 'Admin');
|
|
|
|
INSERT INTO workspace_settings (workspace_id, datatable) VALUES ('wm-fork-dt', '{
|
|
"datatables": {
|
|
"main": {"reference": {"workspace_id": "test-workspace", "datatable": "main"}}
|
|
}
|
|
}'::jsonb);
|