fix(worker): stop leaking credential secret trees and allow multi-line secret entry #351

Merged

rwachters merged 2 commits from fix/credential-secret-tree-removal into master

2026-10-26 16:09:52 +00:00

rwachters commented

2026-10-26 16:08:07 +00:00

(Migrated from github.com)

Summary

Two independent changes: a fix for a defect in the already-merged credential attach feature, and a small app usability change that lets operators enter multi-line secrets.

Secret trees were never actually removed

/run/torvian/secrets is owned torvian:torvian with mode 0711, so the credential account can traverse it but not write to it. rmdir(2) requires write permission on the parent directory, so run-as-vault --clear-secret-dir always failed when it reached its final rmdir of a <token> entry: it had already dropped to the vault account by then, the call returned EACCES, and the helper exited with code 14. Tree contents were unlinked successfully because the tree directory itself is 0700 vault, so no secret material survived; the empty directories accumulated instead, and the startup sweep warned about each one on every restart.

The helper now only empties a tree: remove_tree takes a remove_self flag, which is 0 for the root of the walk and 1 for recursive children, so no rmdir is attempted on the tree root. Validation still runs before drop_privileges, and only the bounded unlink walk runs after it.
The worker deletes the emptied directory itself, because it owns the secret root and therefore holds the parent write permission that removal requires.
Removal fails closed: a DirectoryNotEmptyException or any other IOException becomes a typed PreparationFailed, and no recursive delete is ever attempted.
A failed worker-side removal is now logged at WARN from the material close paths instead of being discarded until the next startup sweep.
deploy/worker/README.md documents the split ownership of the secret root.

Deletion is tied to the profile target's lifetime, not to detach alone: auto-detach on profile exit, the detach action, session close, worker shutdown, attach failure or cancellation before registration, and the startup sweep all converge on the same two-step removal.

Multi-line credential secrets

The credential secret value field was declared singleLine = true, so a long secret could not be inspected and a truncated paste would have been invisible. It is now a bounded multi-line field (minLines = 1, maxLines = 8) with masking, the show/hide toggle, the reveal-state keying and the byte-limit hint all unchanged. The value still reaches the request body untransformed; only reference is trimmed, which is pre-existing behaviour.

This is a usability and risk-reduction change, not a bug fix: the failure that prompted it was a passphrase-protected private key, which OpenSSH cannot unlock non-interactively. The stored value was intact.

Verification

./gradlew :worker:test — 799 tests, 0 failures, 4 skipped.
./gradlew :app:desktopTest — 1120 tests, 0 failures, 2 skipped.
Manual, on a rebuilt image: attaching a file-channel profile no longer leaves a directory behind under /run/torvian/secrets, and the startup sweep is silent after a restart.
Manual, in the app: a pasted multi-line secret can be revealed, saved, and delivered to a file-channel profile with its line structure intact.

Limitations

The C helper is only compiled inside the Docker image, so its change is verified by an image rebuild and manual end-to-end testing rather than by an automated test in this repository.
The real-helper tests remain gated/skipped.
The app has no Compose UI tests for the credential settings tab, so the multi-line field change is verified manually.

## Summary Two independent changes: a fix for a defect in the already-merged credential attach feature, and a small app usability change that lets operators enter multi-line secrets. ## Secret trees were never actually removed `/run/torvian/secrets` is owned `torvian:torvian` with mode `0711`, so the credential account can traverse it but not write to it. `rmdir(2)` requires write permission on the parent directory, so `run-as-vault --clear-secret-dir` always failed when it reached its final `rmdir` of a `<token>` entry: it had already dropped to the `vault` account by then, the call returned `EACCES`, and the helper exited with code 14. Tree contents were unlinked successfully because the tree directory itself is `0700 vault`, so no secret material survived; the empty directories accumulated instead, and the startup sweep warned about each one on every restart. - The helper now only empties a tree: `remove_tree` takes a `remove_self` flag, which is `0` for the root of the walk and `1` for recursive children, so no `rmdir` is attempted on the tree root. Validation still runs before `drop_privileges`, and only the bounded unlink walk runs after it. - The worker deletes the emptied directory itself, because it owns the secret root and therefore holds the parent write permission that removal requires. - Removal fails closed: a `DirectoryNotEmptyException` or any other `IOException` becomes a typed `PreparationFailed`, and no recursive delete is ever attempted. - A failed worker-side removal is now logged at WARN from the material `close` paths instead of being discarded until the next startup sweep. - `deploy/worker/README.md` documents the split ownership of the secret root. Deletion is tied to the profile target's lifetime, not to `detach` alone: auto-detach on profile exit, the `detach` action, session close, worker shutdown, attach failure or cancellation before registration, and the startup sweep all converge on the same two-step removal. ## Multi-line credential secrets The credential secret value field was declared `singleLine = true`, so a long secret could not be inspected and a truncated paste would have been invisible. It is now a bounded multi-line field (`minLines = 1`, `maxLines = 8`) with masking, the show/hide toggle, the reveal-state keying and the byte-limit hint all unchanged. The value still reaches the request body untransformed; only `reference` is trimmed, which is pre-existing behaviour. This is a usability and risk-reduction change, not a bug fix: the failure that prompted it was a passphrase-protected private key, which OpenSSH cannot unlock non-interactively. The stored value was intact. ## Verification - `./gradlew :worker:test` — 799 tests, 0 failures, 4 skipped. - `./gradlew :app:desktopTest` — 1120 tests, 0 failures, 2 skipped. - Manual, on a rebuilt image: attaching a `file`-channel profile no longer leaves a directory behind under `/run/torvian/secrets`, and the startup sweep is silent after a restart. - Manual, in the app: a pasted multi-line secret can be revealed, saved, and delivered to a `file`-channel profile with its line structure intact. ## Limitations - The C helper is only compiled inside the Docker image, so its change is verified by an image rebuild and manual end-to-end testing rather than by an automated test in this repository. - The real-helper tests remain gated/skipped. - The app has no Compose UI tests for the credential settings tab, so the multi-line field change is verified manually.