docs(1408): psk_file reviewed and CLOSED — its one argument was false - #1417
Merged
Merged
Conversation
The proposal's load-bearing sentence does not hold: I claimed `LoadCredential=` delivers a path 'so it needs exactly this field and nothing else'. It composes with the EXISTING env reader today with zero code — ExecStart=/bin/sh -c 'OPENPULSE_CONTROL_PSK=$(cat "$CREDENTIALS_DIRECTORY/control-psk") exec ...' so psk_file does not unlock systemd credentials, it removes a one-line wrapper. And no systemd deployment exists here: zero .service/.timer/.socket files, no packaging, every launch is `nohup ... &` over ssh, deploy-rpi-pair.sh rsyncs to ~/bin and stops. Nothing in the repo SETS OPENPULSE_CONTROL_PSK (control: the reader is found in 3 Rust files), confirming #1234 from a second direction. My 'same security tier' claim holds — and that is what closes it. Relative to the keystore it is the same tier with moving parts removed; relative to env it is the same tier with moving parts ADDED. The '~20 lines' was the parser branch only: realistically 150-250 lines across ~8 files, including a test seam, since load_control_psk has zero tests today. Recorded with a checkable reopen condition rather than deleted. Verification-objective: a proposal must be judged against what the deployment actually does, so that a capability claimed for a mechanism nobody runs is not mistaken for a reason to add code. Review: docs/dev/reviews/review-1408-psk-file.md Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review artifact only. No code, no config field. #1408 is closed; this is the record of why, with a checkable reopen condition.
The load-bearing sentence was false
I claimed
LoadCredential=delivers a path, "so it needs exactly this field and nothing else". It does not — a systemd credential composes with the existing env reader today, with zero code:So
psk_filedoes not unlock systemd credentials; it removes a one-line wrapper. That was the proposal's sole claimed new capability.And there is no systemd deployment here
Verified independently: 0
.service/.timer/.socketfiles and no packaging directory; every launch isnohup … &over ssh as an interactive user (6 scripts);deploy-rpi-pair.shrsyncs binaries to~/binand stops; control binds127.0.0.1. Nothing in the repo setsOPENPULSE_CONTROL_PSK— control: the same filter finds the reader in 3 Rust files. That confirms #1234's "no deployment has ever set the PSK" from a second direction.Two further qualifiers: the daemon needs the operator's PipeWire session, so the natural unit is a user unit — and encrypted credentials for per-user managers arrived only in systemd v256 (trixie yes, the bookworm legacy Pi image no). And a Pi has no TPM2, so "encrypted at rest" is the host key on the same SD card: protection against a non-root same-host reader, which
0600already gives, and none against card theft — beside an Ed25519 station seed that is plaintext at 0600.My "same security tier" claim holds — and that is what closes it
Relative to the keystore it is the same tier with moving parts removed. Relative to env it is the same tier with moving parts added: a path, a permission check, a precedence rule. The two real deltas are small and neither is a capability — env is inherited by children (the daemon's only spawn is
nvidia-smiper GPU tick, absent on a Pi), andvalidate_owner_onlywould refuse a0644file where systemd accepts a0644EnvironmentFilesilently.The "~20 lines" estimate was the parser branch only
Both config templates' comments become false;
inert_psk_key_id_warningsays "OPENPULSE_CONTROL_PSKonly" and a test asserts that substring; the fail-closed error names only the env var; the parser embeds the variable name in its error strings; plus both-present refusal, missing-path error, empty-string-unset, four places in the book, and tests —load_control_pskhas zero tests today, so the file path needs a seam. Realistically 150–250 lines across ~8 files.Recorded for a future attempt
If it is ever built: refuse on both-present — but not for #1234's reason, which does not transfer. Under file-wins a stale
exportis inert, so the dangerous rotation pre-image exists only under env-wins; refusal is right because it is the cheapest rule and legible at start. A set path to a missing file must beErr, neverOk(None).psk_file = ""means unset.Reopen condition: a systemd-managed deployment exists in this repo (a unit file or packaging target) and a non-loopback bind is in use — and even then, try the zero-code
ExecStartwrapper first.Verification
review lint ok · doc frontmatter ok · doc stamps ok · trace check ok. Docs only.
Verification-objective: a proposal must be judged against what the deployment actually does, so that a capability claimed for a mechanism nobody runs is not mistaken for a reason to add code.
Review: docs/dev/reviews/review-1408-psk-file.md
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6