[fix] act on the stack-editor review

[malickyeu]

Summary

Acts on a code + security review of today's merged work (#97, #98, #99). Two
security findings, four quality ones, and one correction to my own wording.

Nothing here shipped in a tagged release — all of it was caught between merge and
tag.

Security

1. The stack editor's backup could be redirected through a symlink. (High)

Saving a CLI-discovered stack's compose file keeps the previous version as
<name>.dc-prev, written with os.WriteFile — which opens with
O_CREAT|O_WRONLY|O_TRUNC and therefore follows a symlink already sitting at
that path. The SSH path had the same weakness twice: cat > "$tmp" and
cp -p src dst both write through a link.

The exploit needs only write access to the stack's directory — not Docker
access, so this was reachable by an account that is otherwise unprivileged (a
deploy user, a CI account). Pre-place compose.yml.dc-prev as a link to
/etc/cron.d/x, put a payload in compose.yml, wait for an operator to hit Save,
and Docker Commander — commonly root — writes attacker-controlled content to an
attacker-chosen path.

Backups now go through writeSiblingAtomic: temp file in the same directory plus
a rename, because rename replaces a symlink instead of following it. The SSH
script uses mktemp (which creates with O_EXCL, so it cannot open a pre-placed
link) and mv.

2. StackRedeploy skipped the containment check that saving enforced. (Medium)

StackWriteComposeFile called checkContained(); StackRedeploy did not. A
compose path outside the stack's working directory was refused for writes but
still handed to docker compose up -d, deploying whatever definition sat there.

Fixed by moving containment into editableReason(), which every entry point
already calls — so the rule can't be enforced on one operation and forgotten on
another. That also fixes a UX bug: StackEditable used to report true for a
target the write path would refuse, putting an editor in front of the user whose
Save could only ever fail.

Correction to my own wording

The comments, docs/stacks.md and the #98 description claimed this guard stopped
"anyone able to run a container" from turning a containers grant into an
arbitrary file write. That was overstated. Setting compose labels requires
direct Docker API access, and the app's CreateSpec exposes no Labels field —
so the attacker must already have root-equivalent access to that host. The rule is
worth having as defence in depth; it is not a barrier against an escalation
reachable through Docker Commander. Reworded everywhere it appeared.

Quality

• compat.yml declared a versions dispatch input and never used it — the
  matrix was hard-coded, so dispatching with ["24"] silently ran all five. Now
  resolved through a choose job and fromJSON.
• evilStack captured t.Context(), which Go cancels just before t.Cleanup
  runs, so every cleanup closure using it failed silently — six leaked containers
  per run. Now takes the background context. Verified: 0 before, 0 after.
• Two resolveStack calls per compose view (one for content, one for
  editability), each a container list plus a file read — an SSH round trip each.
  Collapsed into one StackCompose.
• Redeploy output was set twice, the second overwriting the first.

Type of change

• Bug fix
• New feature
• Docs only
• Refactor / chore

Checklist

• go test -short ./... and go vet ./... pass
• gofmt gate is clean (gofmt -l $(git ls-files '*.go') after staging)
• Frontend type-checks (cd web && npx tsc --noEmit)
• Rebuilt and committed web/dist
• Added/updated tests for the change
• Updated docs/ and added a CHANGELOG.md entry

Notes for reviewers

Both security fixes were mutation-tested against the vulnerability itself, not
just asserted:

• TestWriteSiblingAtomicReplacesASymlink — reverting to os.WriteFile fails it
  with SECURITY: the write followed the symlink and landed in …/victim.
• TestPentestStackRedeploy_IsContainedToo — removing containment from
  editableReason fails it with redeploy accepted a compose file outside the stack's working directory.

That second mutation run is also how the leak was found. With the guard
removed the attack genuinely succeeded, compose up -d started
dctest_evilredeploy-pwned-1, and it survived the run — poisoning the next one,
because ListStacks groups by label and a stale container contributed its
now-deleted config_files path to the same project. So the pentest now force-
removes the whole project label before and after (freeStack), not just its own
fixture container. A pentest whose guard fails leaves real side effects; its
cleanup has to cover the attack's output, not only the fixture's.

TestStackEditableReason now uses real directories rather than fabricated
paths, since editableReason reaches the filesystem. It gained the two
out-of-project cases.

Residual window, stated rather than hidden: the backup is renamed into place
before the new content is. If that second rename fails — same directory, same
filesystem, so realistically only ENOSPC/EIO — .dc-prev has been updated without
the edit being applied. Closing it entirely would mean applying the edit before the
backup exists, which trades a worse failure for a rarer one. Left as is,
deliberately.

Test data: verified the docker suite leaves zero dctest* containers behind
across consecutive runs.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.