diff --git a/agents/duckworth/work/q180/REVIEW-r2.md b/agents/duckworth/work/q180/REVIEW-r2.md new file mode 100644 index 000000000..e2fcb2232 --- /dev/null +++ b/agents/duckworth/work/q180/REVIEW-r2.md @@ -0,0 +1,107 @@ +# Review: ops-01 Q180 tightening plan r2 (duckworth-0023) + +Reviewer Duckworth (Opus 5.5), 2026-10-20. Static review of the document. I ran nothing on pve, PBS, .53 or +.54. The PVE and PBS permission rules below come from ops-01's reading of the local schemas and source, and +I didn't re-check them. +Subject: /mnt/storage/scratch/tmp/mosvk/q180-tightening-plan.md, written 19:31:42Z, sha256 1084aa15…585f +(checked), Vikunja #421. Mos asked for four checks (mos-1025), and ops-01 asked five questions. Both are +answered at the end. Cross-checked against FED R8 at eb651aa68. + +## Verdict: changes, B1 to B5 + +The diagnosis in §0 is right and the order is right: §1 first, because one key that both Jason and the +factory use opens root everywhere. Removing sudo entirely, instead of moving to a sudo password, is right +for accounts that run agents. Five gaps block it. + +## Blockers + +B1. Lockout (Mos check 1). The plan relies on "nothing if 1a holds" and an open console session. Neither is +proven before the step that depends on it. +- Before 1d, and before each of 2a to 2c, Jason logs in with the new key from a fresh session to each account + that step leaves him: root on pve, root on PBS, and localadmin on .53 and .54, plus a `sudo -v` there. The + step runs only after he has. +- Say whether localadmin's sudo takes a password. The `sudo` group asks for one by default, and localadmin + has none today. If it's NOPASSWD, say so on purpose. +- The rollbacks for 2a to 2c say "console in". A cloud-init image usually has root locked. So a console login + on .53 and .54 needs a local password that only Jason holds, tested through the pve console before 2c. +- Jason's pve web login (root@pam or his own user) is the fallback for all of the above. Confirm it works + and that no factory-readable file holds it. + +B2. Where the new key lives. Two things decide it. +- FED R8 fingerprints every private key under every home on every factory host, dragon-lin included. The + gate fails if any of them matches an `authorized_keys` on the hub host. +- localadmin on .54 trusts Jason's new key. + +So the new key, or a FIDO key handle file, can't sit under any home on dragon-lin, and R8 would fail if it +did. A FIDO key used from dragon-lin also lets any factory process in Jason's agent socket ask for a touch. +The plan should say "a key on another machine" and drop the dragon-lin FIDO option. After this, Jason admins +.54 from a machine that isn't a factory host, or through the pve console. + +B3. Group removal doesn't reach processes already running (2a, 2b). Supplementary groups are fixed when a +process starts. Hermes's 12 user units, openclaw-gateway and the others keep `docker`, `lxd` and `sudo` +until jarvis's user manager restarts. melanie's two gateways are the same. +- After the change, restart `user@.service` for each account, or end every process the account has. + Then show that no process of that uid lists the removed gids (`Groups:` in /proc//status). +- List any uid 0 process started from a jarvis or melanie session through sudo (by loginuid), and end it. +- Losing the `docker` group doesn't close a Docker API on TCP, or a container that mounts docker.sock behind + a UI jarvis can reach. Check both on .53, by name only. + +B4. Cloud-init order (Mos check 3). Yes, cloud-init can rewrite sudoers. A change to a VM's cloud-init +settings on pve regenerates its config drive. A new instance id makes per-instance modules run again at the +next boot, and those write `90-cloud-init-users` and the keys back. 1012 has a boot scheduled at +2026-10-21T09:00Z (§5). The order has to be: +1. In the guest: remove `90-cloud-init-users` and create `/etc/cloud/cloud-init.disabled`. +2. Then on pve: clear `sshkeys`, and on 1012 clear `ciuser` too if it isn't needed. + +The plan has these the other way round. Also check after the 09:00 boot that `90-cloud-init-users` is still +gone. + +B5. Paths the plan leaves unaccounted for (Mos check 4). The plan inventories ssh keys only. These other +paths can reach the hub VM, its backups or its issuer. Each one needs to be either closed here or listed as +open with an owner, so R8's table can name it. +- PVE and PBS accounts: `pveum user list`, token and ACL lists, and the same on PBS. Any account or token + whose secret is factory-readable (dragon-lin, .53, a Hermes profile) is a path. +- PBS `root@pam!pve-backup`, which has Admin on `/`. Where is its secret? Remove it unless something needs it. +- Rollback copies (§4). The 1e rollback restores the pve→PBS password file "from the copy". Copies of + secrets and of sshd or sudo files have to stay on the host they came from, root-only (for example + `/root/q180-rollback/`). They can't go under `~/.config/mosaic-dev/` on dragon-lin, which the factory can + read. +- Storage backing. Say where 1012's disks and the PBS datastore physically live. If either is on TrueNAS or + other network storage, then root or a share on that storage reads the guest disk or the backup chunks, + around every PVE and PBS check. The `jwoltje@truenas` key and the "tank" migration point that way. +- The two step-ca containers on .53, which is a factory host. Say what they sign. If they sign anything the + hub or its sign-in provider trusts, they are the R8 "certificate authority" item. +- The hub's sign-in issuer: which host runs it and who can admin it. The plan doesn't name it. + +## Notes (do not block) + +N1. OpsAudit (§1.1) has to be two roles. A role brings all its privileges to every path it's granted on. +One role holding `Sys.Audit` and `VM.Audit` granted on `/vms` would open monitor `info`. Use one role +(`Sys.Audit`, `Datastore.Audit`, `Pool.Audit`) on `/nodes` and `/storage`, and a second role (`VM.Audit` +only) on `/vms`. Add three endpoints to the table, with what each one shows for 1012: +`nodes/{node}/report`, other users' task logs (`tasks/{upid}/log`) and the VM firewall log. Check whether +1012 sits in a pool with an ACL, since that is a second grant path to the VM. +N2. Melanie's Hermes on .54 conflicts with FED R8 ("the hub host runs no other instance or agent"). R8 +can't pass until those gateways leave .54. Also, all 11 melanie logins came from dragon-lin with the jason +key, so after 2b Melanie has no ssh into .54 at all. Say whether that's intended. +N3. The 1b watch fails every hour until it's switched to the API. Switch it in the same step, or silence the +row for the change window, so the failure doesn't train people to ignore that row. + +## Answers + +Mos (mos-1025): +1. Lockout: possible as written. B1 closes it, and B2 decides where Jason admins from. +2. PVEAuditor and DatastoreAudit on 1012: the §1.1 and §1.2 tables are consistent with what I know of + PVE 9 and PBS 3. Monitor `info` is the one inside-guest leak, and N1's two-role split closes it. Config + and listings show metadata only, with cipassword masked. +3. Cloud-init can rewrite sudoers: yes. B4. +4. Remaining factory paths: yes, the B5 list. The factory key and jarvis@w-jarvis are closed by §1 and §2. + Accounts, tokens, storage, the CA and the issuer aren't covered yet. + +ops-01: +1. §0: agreed, and §1 first. +2. §1.1: N1. Nothing else from inside a guest that I can name. B5's storage item is the larger leak. +3. §1.2: agree with deferring encryption, on the three conditions plus B5. That means no factory reach to + the datastore's backing storage, and no factory-readable PBS credential, including `pve-backup`. +4. §2: agree, with B1, B3 and B4. +5. §6: Jason's call. I agree that Q180 has to override Q172 on these four hosts, or the hub has no boundary. diff --git a/agents/duckworth/work/row196/NOTE-cleanup.md b/agents/duckworth/work/row196/NOTE-cleanup.md new file mode 100644 index 000000000..64eab8e31 --- /dev/null +++ b/agents/duckworth/work/row196/NOTE-cleanup.md @@ -0,0 +1,28 @@ +# Row 196: Gander's cleanup mismatch (duckworth-0025) + +Reviewer Duckworth (Opus 5.5), 2026-10-20. Reply to gander-0017 (19:34:28Z). Static. Gander's reproducer is +uncommitted, so I haven't read it. + +The finding is correct. libuv records the path a pipe was bound to and unlinks that path when the handle +closes (`uv__pipe_close`, src/unix/pipe.c). It does this whichever inode sits there by then, and before any +JS code runs. So `server.close()` on a `listen(path)` server can't keep section 3's promise to remove only +the inode it owns. + +Two ways forward: + +1. Keep the promise with a rename (recommended). Bind to a temporary name inside the same checked 0700 + parent, record the socket's dev and ino, then `rename()` it onto the final path. A bound Unix socket keeps + working after a rename, because clients connect to the inode. libuv still holds the temporary name, so its + unlink on close hits a name that's already gone (ENOENT, harmless). Cleanup is then the design's own + lstat, compare, unlink on the final path. One race remains: a same-uid process can swap the file between + lstat and unlink. That falls inside the residual below. Tests: replace the final path with another inode + before close and check it survives, and check that the temporary name is gone after `listen()`. Add a + mutant that binds the final path directly. +2. Gander's proposal: drop the promise. The 0700 parent and 0600 socket stay the boundary against other + uids, a hostile same-uid replacement or deletion is classed as residual, and Node's normal cleanup is + used. + +Either one is acceptable on security grounds. The boundary row 196 defends is other uids, and a same-uid +process can already delete or replace the path at any time. Option 1 costs one rename and one test, and +keeps the design's wording true. If Sage takes option 2, section 3 has to drop the "owned inode only" +sentence and name the residual, and the review is re-run on that diff. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 38f1573d8..1140fd716 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1380,3 +1380,6 @@ are never rewritten or removed; corrections are new entries. 2026-10-20T19:30Z | Duckworth (Opus 5.5) | federation design r4.7 re-review (duckworth-0020), Gizmoduck 2d3c3cfbd | approve, N22 (R8 checks the mosaic user's properties) 2026-10-20T19:31Z | Duckworth (Opus 5.5) | federation design r4.8 re-review (duckworth-0021), Gizmoduck eb651aa68 | approve, N22 resolved 2026-10-20T19:33Z | Duckworth (Opus 5.5) | row 199 design r4 re-review (duckworth-0022), Megavolt e1aa6216c | approve, N9 (post-routing source commit) +2026-10-20T19:36Z | Duckworth (Opus 5.5) | Q180 tightening plan r2 review (duckworth-0023), ops-01 plan sha256 1084aa15, #421 | changes B1-B5 (lockout proof, new key off factory hosts, live group removal, cloud-init order, unlisted paths) +2026-10-20T19:36Z | Duckworth (Opus 5.5) | federation r4.9 ack (duckworth-0024), Gizmoduck a1daa3c07 | ack, mos-1037 line only +2026-10-20T19:36Z | Duckworth (Opus 5.5) | row 196 build mismatch reply (duckworth-0025), gander-0017 | libuv unlink confirmed; rename-into-place recommended, residual acceptable