security: 2026-08-02 audit remediation (33/33), A7 bundle binding, dependency/CI maintenance - #6
Open
beardedeagle wants to merge 17 commits into
Open
security: 2026-08-02 audit remediation (33/33), A7 bundle binding, dependency/CI maintenance#6beardedeagle wants to merge 17 commits into
beardedeagle wants to merge 17 commits into
Conversation
…tries via AAD All findings from the 2026-08-02 security audit dispositioned: - HIGH (5): O1, A1-A4 fixed. - MEDIUM (11): O2-O4, A5-A11 fixed; strict:false investigated and closed (no fail-open beyond A3; --verify-keys is now fail-closed under any skip). - LOW+INFO (16): 12 fixed, 4 documented as accepted in the threat model (A14, A18, A24, A25). A7 (no AAD binding): bundle entries are now name-bound — resolve_bundle decrypts each ciphertext with AAD = the entry's map key, so relocating a ciphertext under a different entry fails authentication. Suite bumped to ecies-secp256k1-hkdf-sha256-aes-256-gcm-entry-aad (pre-AAD bundles fail loudly rather than silently). AAD support comes from the ecies fork branch aad-support (additive encrypt_with_aad/decrypt_with_aad; upstream PR ecies/rs#159), consumed via [patch.crates-io] git+rev pin 81e1b84; deny.toml gains a [sources] allow-git entry for the fork. nix cargoLock.outputHashes is pending a nix-equipped machine (fill-in steps documented in package.nix and nixpkgs-package.nix). Also: docs and threat model brought in line with the new behavior; daemon hardening (signal deferral across the plaintext-write window on macOS, logged hardening failures, fd-pinned writes, O_NOFOLLOW opens, sentinel-guarded bulk cleanup, keychain reads via security-framework). Verified: fmt, clippy (macOS dev + x86_64-linux-gnu + macOS release, -D warnings), full locked test suite, mdbook build.
Fetch hash of the aad-support fork at rev 81e1b84, determined via the fake-hash trick on a nix-equipped machine. Replaces the placeholder fill-in instructions in package.nix and nixpkgs-package.nix.
…tion - keychain reads described as "via security(1)" -> Security.framework API (README, architecture/postmaster.md, getting-started/key-provisioning.md) [A17] - launcher-contract key_var_for: lossy suffix mapping -> hard error for anything outside [A-Za-z0-9_] (no aliasing) [A20] - threat-model fd-closure: drop the "5 sites" count; dotenvy no longer opens its own fds (from_read_iter on postmaster-opened fds) [A13] - dependency policy: 8 -> 10 direct deps, add security-framework (macOS) and a note on the ecies fork git+rev pin (ecies/rs#159) - materialization-abi conformance list: drop the cryptographically unbound "or profile" leg of the replay case - cleanup: document the .postmaster-managed sentinel guard in the cleanup rustdoc, HELP_CLEANUP, Invocation::Cleanup, and cli.md [A19] - ramdisk.rs: drop the obsolete /usr/bin/security analogy mdbook build, fmt, clippy, full locked test suite green.
Investigation note left over from the 2026-08-02 audit session. The issue it describes (SUMMARY.md referencing a missing changelog.md) is resolved by the just docs recipe, which copies the root CHANGELOG.md into docs/src before mdbook build and removes it after.
- base64 0.22 -> 0.23 (supersedes PR #5; the lock resolves 0.23.1) - cargo update for the minor/patch set: libc 0.2.189, serde family 1.0.229, serde_json 1.0.151, syn 2.0.119 + 3.0.3, plus other semver-compatible bumps (supersedes PR #4) - PRs #1-#3 are moot on this branch: actions/checkout is pinned by sha to exactly v7's commit; actions/cache and DeterminateSystems/nix-installer-action were removed in the workflow rewrite (nix is installed via the official nixos.org script) Verified: fmt, clippy (macOS + x86_64-linux-gnu + release, -D warnings), full locked test suite, cargo deny, cargo audit.
This was referenced Aug 5, 2026
This was referenced Aug 5, 2026
There was a problem hiding this comment.
Pull request overview
Implements the August 2026 security-audit remediation, including bundle AAD binding, runtime hardening, dependency updates, and CI maintenance.
Changes:
- Adds fail-closed validation, cleanup, key verification, and file-operation hardening.
- Binds bundle ciphertexts to entry names and reorganizes launcher modules.
- Updates dependencies, Nix packaging, CI workflows, build tooling, and documentation.
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
tests/exec_conformance.rs |
Tests partial key-resolution failure. |
tests/exec_check.rs |
Updates config import path. |
src/server.rs |
Hardens reads, directories, and processes. |
src/secret_files.rs |
Adds secret writing and pruning. |
src/ramdisk.rs |
Extracts macOS RAM-disk verification. |
src/lib.rs |
Exposes reorganized modules. |
src/launcher.rs |
Adds signal deferral and cleanup handling. |
src/keys.rs |
Tightens key loading and failure tracking. |
src/key_source.rs |
Extracts key-source selection. |
src/key_provisioning.rs |
Extracts setup and cleanup operations. |
src/exec_config.rs |
Extracts exec configuration validation. |
src/config.rs |
Tightens daemon config parsing. |
src/bundle.rs |
Adds entry-name AAD binding. |
src/adapter.rs |
Rejects ambiguous environment names. |
README.md |
Updates build and keychain guidance. |
package.nix |
Adds the git dependency hash. |
nixpkgs-package.nix |
Adds the git dependency hash. |
Justfile |
Adds docs and build-matrix recipes. |
docs/src/reference/launcher-contract.md |
Updates launcher behavior documentation. |
docs/src/reference/cli.md |
Updates verification and cleanup guidance. |
docs/src/getting-started/quick-start.md |
Updates dotenvx key instructions. |
docs/src/getting-started/key-provisioning.md |
Updates macOS keychain guidance. |
docs/src/architecture/threat-model.md |
Documents remediations and residual risks. |
docs/src/architecture/postmaster.md |
Updates dependency architecture. |
docs/src/architecture/materialization-abi.md |
Documents the AAD suite. |
deny.toml |
Allows the pinned ecies fork. |
CHANGELOG.md |
Marks version 0.1.0. |
Cargo.toml |
Updates dependencies and release hardening. |
Cargo.lock |
Locks updated dependency versions. |
.github/workflows/security.yml |
Consolidates security tooling setup. |
.github/workflows/nix.yml |
Reworks Nix installation and checks. |
.github/workflows/docs.yml |
Reworks documentation CI. |
.github/workflows/ci.yml |
Consolidates Rust quality checks. |
.github/SECURITY.md |
Improves reporting links. |
.github/CODE_OF_CONDUCT.md |
Adds contributor conduct policy. |
.github/actions/setup-rust/action.yml |
Adds shared Rust/tool setup. |
Suppressed comments (1)
.github/workflows/docs.yml:55
just docsinvokesmdbookdirectly, but neither this workflow norsetup-rustinstalls mdBook. Installing Nix alone does not putnixpkgs#mdbookon PATH, so the build step fails withmdbook: command not found. Run the recipe inside a Nix shell containing mdBook.
- name: Build book
run: just docs
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…truth) Removes doctrine violations introduced during the 2026-08-02 audit remediation: - new in-repo composite .github/actions/setup-nix: downloads the version-pinned official Nix installer (2.34.7), verifies it against a committed sha256 BEFORE executing (fails closed on mismatch), enables flakes, and appends nix to GITHUB_PATH for later steps (the latter fixes a latent bug: a re-enabled job would have failed with "nix: command not found") - nix.yml and docs.yml drop the inline unpinned curl|sh install and consume the composite - Justfile gains flake-check-eval-all; nix.yml's eval-all-systems step now runs the just recipe instead of a raw nix command No third-party actions anywhere; the only action used remains actions/checkout, pinned by sha. Verified: checksum logic both ways, YAML lint on all automation files, just parses both flake recipes.
Pure code motion per the file-size convention (500 soft / 750 danger /
1000 hard); zero behavior change.
- keys.rs (902) -> keys/{mod,load,env,keychain,verify}.rs; KeyRing's
single inherent impl stays in keys/mod.rs (clippy::multiple_inherent_impl
is deny crate-wide) with bodies delegating to pub(super) free fns
- key_source.rs -> keys/source.rs; key_provisioning.rs (659) ->
keys/provisioning/{mod,setup,cleanup}.rs
- config.rs (581) -> cli/{mod,help}.rs
- adapter.rs -> adapter/mod.rs; bundle.rs -> adapter/bundle.rs
- tests/exec_conformance.rs (733) -> tests/exec_conformance/{main,
env_mode,files_mode,keys_verify,bundle}.rs (single test target, same
target name, same 16 tests)
- Path stability: lib.rs re-exports (pub use cli as config; pub use
keys::{source as key_source, provisioning as key_provisioning}; pub use
adapter::bundle) — zero call-site changes in launcher/server/etc.
- docs: file-path references updated (launcher-contract.md, cli.md)
Verified: fmt, clippy (macOS + x86_64-linux-gnu + release, -D warnings),
identical test totals (75 lib + 7 exec_check + 16 exec_conformance),
cargo deny, cargo audit, mdbook build.
Modeled on citadel's docs/src/unsafe-code/: an index with the exception policy and a registry table, plus one page per site group — - signal-deferral: TerminationGuard sigmask/pending/re-raise/Drop (macOS) - macos-libc-boundary: getpeereid(2) peer check + PT_DENY_ATTACH hardening - systemd-fd-ownership: UnixListener::from_raw_fd adoption (Linux) - test-env-mutation: set_var/remove_var on edition 2024, incl. the known per-module-lock gap (tracked separately) Also corrects the [lints.rust] unsafe comment in Cargo.toml: mlockall / setrlimit / set_dumpable are safe rustix wrappers, not unsafe; the actual macOS hardening unsafe is PT_DENY_ATTACH; the comment now points at the new docs section. mdbook build, fmt, clippy, full test suite green.
Per citadel's lint set. The clippy lint is warn-level in Cargo.toml; it fires on exactly one duplicate: syn 2 (pinned by wasm-bindgen, which is target-gated to wasm32-unknown-unknown inside the ecies dep and never compiled for postmaster's real targets) vs syn 3 (serde_derive). The lint has no per-package scoping and the case is uncontrollable until upstream wasm-bindgen moves to syn 3, so lib.rs and main.rs carry a documented crate-root allow, and deny.toml gains [bans] multiple-versions = "warn" with syn skipped — ongoing visibility for convergence and for any NEW duplicates. Verified: fmt, clippy (macOS + x86_64-linux-gnu + release, -D warnings), full test suite, cargo deny.
…idual Security code: - secret_files: post-open write errors unlink the target via the pinned dir fd; cleanup_written is fd-relative (dir fd + entry names, unlinkat); prune_stale pins the dir once and enumerates/stats/unlinks fd-relative (statat/unlinkat) — no path re-resolution anywhere - launcher: secrets dir opened once (O_DIRECTORY|O_NOFOLLOW|CLOEXEC), shared by write_secret_file and both cleanup paths - cleanup: same fd pinning; tracks failures and only removes the .postmaster-managed sentinel when every managed file was removed (retains it and errors otherwise, so retries aren't refused) - verify_key_sources reuses the runtime key-variable predicate (is_key_var_name) for both key prefixes - uid_of (Linux): id resolved only from verified absolute candidates (/usr/bin/id, /bin/id, /run/current-system/sw/bin/id) — fstat regular file, uid 0, no group/other write - bundle: encoding "utf8" validated at resolution (fail closed) - daemon Entry: deny_unknown_fields (nested, matches root) - HELP_EXEC + Invocation::Exec rustdoc describe the any-skip-aborts --verify-keys semantics Automation: - setup-nix: invoke the official installer correctly (--daemon --yes --no-modify-profile) - setup-rust: enforce the just version pin (install on mismatch) - Justfile: docs-serve uses EXIT trap + subshell; build-matrix applies target-cpu=native only for the native target (arm64|aarch64) and no override for explicit ARM targets; build-all actually builds all six targets; docs/src/changelog.md untracked + gitignored (generated by the docs recipe) Docs: - CHANGELOG gains an Unreleased section for this PR - unsafe-code/signal-deferral documents the pending-check/exec race as an accepted residual (irreducible below fork+exec or a handler+flag design; bounded worst case) Rebutted: the config-compat thread (the types named never lived in config.rs; pub use cli as config preserves every path; battery green). Verified: fmt, clippy (macOS + x86_64-linux-gnu + release, -D warnings), 82 lib + 7 exec_check + 16 exec_conformance tests, cargo deny, cargo audit, mdbook build.
The unsafe-code section documents unsafe *usage* (UB vectors and their mitigations) only. The pending-check/exec window is a design-level logic race in how the launcher uses the guard, not a property of the unsafe blocks — it belongs in the threat model's plaintext-persistence section, where it now lives. signal-deferral.md keeps a cross-reference only; the PR thread comment was updated to match.
…ow-up) The pending-check/exec handoff had a documented residual: a termination signal arriving between the final sigpending check and the mask restore was delivered at restore — before execvp, before cleanup — stranding plaintext on the ramdisk. Microseconds-wide, but nonzero. The guard now disarms by installing a flag-recording handler while the signals are still blocked, then restoring the mask: anything pending from the window is delivered to the handler (a single atomic store — the canonical async-signal-safe operation), never to the default disposition, so no termination signal can kill the launcher without cleanup_written running first. sigaction rc failures fall back to the previous pending-check behavior with a log line (fail closed into the old behavior, never silently weaker). This is not a new unsafe surface: the sigaction install is the same libc signal-operation family, same file, same invariants as the four blocks that already existed — the surface grows in size, not in kind. One residual remains, of a different kind: a signal landing between the final flag check and execvp is recorded but unexamined, so the launch proceeds and the manager's escalation performs the stop — supervision-semantics, not plaintext exposure (threat model updated). - deterministic test: disarm_records_signal_raised_during_the_window (raise SIGTERM at self mid-window; flag observes it; process survives) - docs: unsafe-code/signal-deferral rewritten for the new mechanism (UB table, handler-purity registry rule, fallback), threat-model residual replaced, CHANGELOG + registry index updated Verified: fmt, clippy (macOS + x86_64-linux-gnu + release, -D warnings), 83 lib + 7 exec_check + 16 conformance tests, cargo deny, cargo audit, mdbook build.
Narrows the unexamined-consumption sliver from the disarm->Command-build window (~5-50us) to the execve path itself (~1-10us): the flag is now read at disarm and again immediately before exec, and a recorded signal triggers cleanup_written + die_by instead of being carried silently into the child (A11 follow-up). Verified: fmt, clippy (macOS + x86_64-linux-gnu + release, -D warnings), 83 lib + 7 exec_check + 16 conformance tests, mdbook build.
…cleanup on every death path Per the sibling-monitor design (author-approved 2026-08-05; rulings: 250 ms grace constant, retire the flag-handler, pidfd+kqueue liveness, monitor on all exec launches). - The launcher forks after zeroization, before execvp: the parent execs the service (MainPID unchanged), the fork-child becomes the monitor. - The monitor keeps the four termination signals blocked for life and consumes them synchronously (sigpending/sigwait) — no signal handlers anywhere; the sigaction flag-handler and second pre-exec gate are retired. - Stop policy: forward -> 250 ms grace -> SIGKILL -> cleanup -> exit. Service death from ANY cause (natural exit, kill -9, crash) -> cleanup -> exit — including interactive kill -9, which previously had no cleanup path outside systemd ExecStopPost. - Liveness: pidfd+poll on Linux (pidfd_send_signal for forwarding — PID-reuse-proof), kqueue NOTE_EXIT on macOS; kill(pid,0) polling is fallback-only. - The monitor scrubs the inherited env-pair heap at start (into_vec + zeroize in place): free() alone would leave plaintext in mapped pages for the monitor's whole life. - fork failure fails closed — no unsupervised launch. - The Linux TerminationGuard leg is now real (rustix runtime rt_sigprocmask); rustix runtime/event features adopted (experimental API, lockfile-pinned, fails loudly on change); libc::fork is the single libc FFI call on the Linux exec path (doctrine exception, documented in the registry, monitor.md, postmaster.md, CHANGELOG). Tests: four new monitor tests run the real loop in real forked processes (natural death, forwarded TERM, TERM-ignoring service SIGKILLed after grace, env-mode no-op) plus the wipe_pair scrub test; two conformance tests updated to the new contract (plaintext removed on service death; 0600 verified in-service). 87 lib + 7 exec_check + 16 conformance, zero regressions. Docs: new unsafe-code/monitor.md; signal-deferral rewritten for the reduced guard; registry, threat model (monitor model + inherited-heap mitigation), postmaster.md Stage 5 + dependency policy, CHANGELOG, SUMMARY. Verified: fmt, clippy (macOS + x86_64-linux-gnu + release, -D warnings), full test suite, cargo deny, cargo audit, mdbook build.
Three separate disciplines raced: module-local ENV_LOCK statics in keys/source.rs and keys/provisioning/mod.rs, and no lock at all in keys/env.rs. Module-local locks do not exclude each other, so two env-mutating tests still raced (~1 in 5 baseline runs); the loser's panic poisoned a mutex and failed unrelated key_source tests. - new src/test_util.rs (cfg(test)) with the single process-wide crate::test_util::ENV_LOCK; both module-local statics deleted - every env-mutating test now holds it for the mutation's full scope, including keys/env.rs's previously-unlocked from_env test - provisioning skip-guard now covers only ambient runner env; a concurrent test can no longer be the source - unsafe-code docs: test-env-mutation.md rewritten to the process-wide discipline; index rationale line corrected (plaintext custody is minimized by design; unsafe code is where that can silently break) Verified: fmt, clippy x3 (dev/release/x86_64-linux-gnu), 87 lib + 7 exec_check + 16 conformance, deny, audit --deny, mdbook; 15/15 clean lib-suite runs as the flake shake.
load_ring hardcoded KeyNaming::Env at every source leg, so POSTMASTER_KEY* material was dropped (zeroized) at parse time and every bundle_files config failed at exec with "key ring has no POSTMASTER_KEY entry", while setup --recheck accepted either prefix and reported OK. Unit/conformance tests never saw it: they build rings directly, bypassing load_ring. - ExecConfig::required_namings() derives the adapter set from the config (env_files -> Env, bundle_files -> Bundle, both -> both) - from_pairs/load/from_env/from_keychain/load_ring take &[KeyNaming] and admit exactly the required prefixes; unrequired material is dropped+zeroized at parse even when the source offers it - verify_key_sources environment leg now requires EVERY required naming present (was: either prefix) — kills the setup-OK/exec-fails lie in both directions - daemon keeps [Env] with a documenting comment; bundle is not in the fetch contract (conscious exclusion) - regression coverage at the broken seam: load_ring unit tests (admits-bundle-when-required, excludes-unrequired), from_pairs and from_env multi-naming tests, per-naming verify tests, and two end-to-end conformance tests driving the real binary (bundle-only and mixed env+bundle) in tests/exec_conformance/bundle.rs Verified: fmt, clippy x3 (dev/release/x86_64-linux-gnu), 92 lib + 7 exec_check + 18 conformance, deny, audit --deny, mdbook; 10/10 full-suite flake shake.
prune_stale and the sibling monitor's cleanup delete files based on one service's key set, so two services sharing one secrets_dir lets one service's lifecycle delete the other's live files (availability, never confidentiality). The generated Nix modules already namespace per service — Linux units mount a namespace-private tmpfs each, the darwin module writes per-service subdirectories — so the rule binds hand-written configs only. Runtime enforcement via a service-identity sentinel is considered-and-deferred to the broker era.
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.
Summary
Single branch carrying the full 2026-08-02 security-audit remediation plus dependency/CI maintenance; supersedes the five open dependabot PRs.
--verify-keysis now fail-closed under any skip); LOW+INFO: 12 fixed, 4 documented as accepted in the threat model.resolve_bundledecrypts each ciphertext with AAD = the entry's map key, so a ciphertext relocated under a different entry fails authentication. Suite bumped toecies-secp256k1-hkdf-sha256-aes-256-gcm-entry-aad(pre-AAD bundles fail loudly). AAD support comes from the additiveencrypt_with_aad/decrypt_with_aadon the author's ecies fork, consumed via a git+rev[patch.crates-io]pin (81e1b84); upstream PR: Add optional AAD support to encrypt/decrypt ecies/rs#159.deny.tomlgained a[sources] allow-gitentry for the fork.cargoLock.outputHashesfilled for the git-pinned dep;just flake-checkverified green end-to-end from a clean clone on an aarch64-darwin machine.security(1)),key_var_forcontract (hard error, no lossy mapping), fd-closure section, dependency policy (10 direct deps), conformance list (dropped the unbound "or profile" leg), cleanup sentinel guard documented, and more. User-facing docs, help text, and rustdoc are now consistent with the code.cargo update(libc 0.2.189, serde family 1.0.229, serde_json 1.0.151, syn 2.0.119 + 3.0.3).On the superseded dependabot PRs
actions/checkoutby sha to exactly v7's commit (3d3c42e).nixos.orginstall script.Verification
fmt, clippy (macOS dev + x86_64-linux-gnu + macOS release,
-D warnings), full locked test suite,cargo deny,cargo audit, mdbook build, andnix flake check— all green.Known issue (pre-existing, not addressed here)
Bundle mode is broken end-to-end, and was before this work:
load_ringhardcodesKeyNaming::Envfor every key source, soPOSTMASTER_KEY*material never enters the key ring andresolve_bundlefails through the real exec path (unit/conformance tests pass because they build rings directly). Held for a design decision (adapter-aware naming vs. admit-both-prefixes) before a fix.