fix(48): keep launcher pid and log files out of world-writable /tmp - #54
Merged
arena-ai-coding-agent[bot] merged 2 commits intoSep 25, 2026
Merged
Conversation
A minted launcher defaulted to `/tmp/<app>-server.pid` and
`/tmp/<app>-server.log`. `/tmp` is world-writable and, on most
distributions, not guaranteed to be cleared only of its owner's files, so
the default put a predictable, pre-creatable path in a directory every
user on the host can write. Hypatia's patrols 82 and 83 are the two
findings this closes.
Defaults now resolve per user, in the shell, at the point of use:
PID_FILE="${XDG_RUNTIME_DIR:-${XDG_STATE_HOME:-$HOME/.local/state}}/..."
LOG_FILE="${XDG_STATE_HOME:-$HOME/.local/state}/..."
The PID prefers `$XDG_RUNTIME_DIR` because it is the one XDG base
directory with a defined lifetime and documented 0700 permissions; the
log falls to `$XDG_STATE_HOME` because a log outlives the session a
runtime directory describes. Resolution is left to the shell rather than
computed in Rust so the generated script stays portable to hosts where
those variables are set by pam_systemd after login — a path baked in at
mint time would be stale on the next boot.
An explicit `pid_file` / `log_file` in the `[runtime]` block still wins,
unchanged, including its `~` expansion.
The launcher now creates those directories itself: `ensure_state_dirs()`
runs as the first statement of `start_server()`, `mkdir -p`s both
dirnames, and `chmod 0700`s them. Ordering matters — a directory created
after the pid file is written is no protection at all — so a test pins
`ensure_state_dirs` ahead of the first write to `$LOG_FILE` rather than
merely asserting both appear somewhere in the script. `mkdir -p -m` was
deliberately not used: with `-p`, the mode applies only to the deepest
directory created.
Tests pin the emitted `PID_FILE=` and `LOG_FILE=` lines as literals
rather than recomputing them with `format!`, so a future "tidy-up" of
the default expression has to edit the expectation instead of being
confirmed by the same expression it changed.
Verified: `cargo test --all-targets` 90 passing (74 before, floor 59),
`cargo fmt --all -- --check` clean, `cargo clippy --offline --all-targets
-- -D warnings` clean. The currency-locked fixture was re-minted with
the built binary per `fixtures/metadata_block/README.adoc`; the frozen
2026-09-22 artefact was not touched.
Closes #48
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
`REQUIRED_SCALAR_KEYS` did not agree with the launcher standard's `(metadata-block :required-fields …)`, and it disagreed in both directions. It demanded three keys the standard has never asked for, and it accepted a block missing four the standard does require. Both halves are settled here. == The three keys that were not in the standard `runtime-kind`, `standard-spec-version` and `generator` are demoted to `ADVISORY_SCALAR_KEYS`. They are facts about the generator and the run, not about the launcher's contract with the estate, and requiring them is precisely what made a conformant launcher read as broken: `hyperpolymath/trigger`'s launcher carries all eleven fields the standard requires and none of these three, so every release of this tool has called it invalid while the standard called it conformant. `mint` keeps emitting them — they are useful provenance and every existing launcher carries them — but their absence no longer fails the guard. A requirement belongs in the standard, not in a parser. == The four keys that were in the standard `app-url`, `standards-compliance`, `modes`, `platforms` and the two lifecycle-phase lists are now checked. `standards-compliance` is a LIST, so `missing_required` looks at list keys as well as scalars; a scalar-only check could never have found it missing, which made the requirement unfalsifiable as written. The four declarations the emitter never made are now emitted. Their values come from the standard, not from the template: `platforms` and the two lifecycle lists are read out of new `(platforms …)` and `(lifecycle-phases …)` clauses, so the vocabulary is named once for the estate rather than invented per launcher. `modes` is the launcher's own surface — the arms of the generated script's main switch — and is pinned to that switch in both directions: a declared mode with no arm, or an arm with no declaration, fails. ⚠ Finding, not fixed here: the standard's `(required-modes)` also lists `--version`, which the generated script does not implement. That is a genuine gap between the launcher and the standard and is recorded rather than papered over by copying the standard's list into the block. == The encoding is now part of the requirement Until now the block's delimiters and field syntax lived only in `metadata_block.rs`, so a launcher could satisfy every requirement in the standard and still be unreadable by the tool the standard names as its consumer — `hyperpolymath/trigger`'s launcher carries all eleven fields in a `key: value` dialect with no markers at all. The standard now declares `(metadata-block (encoding …))`: both marker pairs (including the retired one, which is what keeps pre-2026-09-23 launchers readable), the comment prefix, the document head, and the field syntax. The parser's constants are asserted equal to the declared strings, and a block built from the deed's own declared encoding is asserted to parse. == Non-vacuity, measured not assumed * `required_keys_are_exactly_the_standard_required_fields` is an equality over two files; alone it is a tautology with extra steps, so `the_old_guard_passed_a_block_missing_four_required_fields` shows the committed 2026-09-22 artefact satisfying the OLD list (kept as a literal) while failing the new one. That contradiction is the defect. * Mutants killed: reverting `REQUIRED_SCALAR_KEYS` to the old list → 1 failure; editing the deed's `:marker-begin` → 1 failure; removing the four `push_list` calls → 6 failures. * `the_three_keysthat_are_not_in_the_standard_are_advisory` strips each advisory key from a complete block and asserts the result still reads as conformant — and asserts the key really is gone, so the strip cannot silently stop stripping. == The vendored standard now differs from canon `standards/launcher-standard_praxis.deed` carries three clauses canon does not have yet (the two value domains and the encoding clause). The content-hash pin that guards against accidental drift was updated in the same commit, and its doc comment records the divergence and why; upstreaming to `hyperpolymath/standards` is pending. Verified: `cargo test --all-targets` 98 passing (was 90), `cargo fmt --all -- --check` clean, `cargo clippy --offline --all-targets -- -D warnings` clean. The currency-locked fixture was re-minted with the built binary; the frozen 2026-09-22 artefact was not touched, and its four missing fields are now asserted by name in the fixture README. Closes #41 Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Contributor
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
arena-ai-coding-agent
Bot
deleted the
arena/01a0da2b-launch-scaffolder
branch
September 25, 2026 21:38
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.
Closes #48.
A minted launcher defaulted to
/tmp/<app>-server.pidand/tmp/<app>-server.log— Hypatia patrols 82 and 83./tmpis world-writable, so the default put a predictable, pre-creatable path where any user on the host can write it.Defaults now resolve per user, in the shell:
PID prefers
$XDG_RUNTIME_DIR(defined lifetime, documented 0700); log falls to$XDG_STATE_HOME(a log outlives a session). Resolution stays in the shell becausepam_systemdsets those after login — a path baked in at mint time would be stale next boot. An explicitpid_file/log_filestill wins, unchanged, including~expansion.The launcher creates those directories itself.
ensure_state_dirs()runs as the first statement ofstart_server(),mkdir -ps both dirnames andchmod 0700s them.mkdir -p -mwas deliberately not used: with-pthe mode applies only to the deepest directory created.Non-vacuity: the tests pin the emitted
PID_FILE=/LOG_FILE=lines as literals rather than recomputing them withformat!, and one test pinsensure_state_dirsahead of the first write to$LOG_FILE— a directory created after the pid file is written is no protection at all.Verified:
cargo test --all-targets90 passing (74 before; CI floor 59),cargo fmt --all -- --checkclean,cargo clippy --offline --all-targets -- -D warningsclean. The currency-locked fixture was re-minted with the built binary perfixtures/metadata_block/README.adoc; the frozen 2026-09-22 artefact was not touched.ℹ️ This branch also carries the #41 commit (see the next PR). #48 is
1250e2b, the first commit on the branch.