Skip to content

Refuse unmounted audit store bring-up (#924) - #948

Merged
sehkone merged 1 commit into
mainfrom
sehkone/issue-924
Aug 28, 2026
Merged

Refuse unmounted audit store bring-up (#924)#948
sehkone merged 1 commit into
mainfrom
sehkone/issue-924

Conversation

@sehkone

@sehkone sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Make bootroot infra up load the required agent configuration, render and verify the audit-store phases, and refuse the stack before Docker starts when filesystem enforcement is not active.
  • Render the audit bind in long form with create_host_path: false, retain short-form reading, and atomically upgrade existing overrides without changing their source path.
  • Update translated operator guidance, release notes, and the registrar E2E invocation for the guarded bring-up flow.

Closes #924. Part of #773. Part of #775.

Test plan

  • scripts/preflight/ci/check.sh (formatting, Clippy, rustdoc, Python/Biome/Markdown checks, docs build, security audit, and Compose validation).
  • scripts/preflight/ci/test-core.sh.
  • bash -n scripts/impl/run-registrar-internal-init-e2e.sh.
  • scripts/preflight/ci/e2e-matrix.sh --skip-hosts was attempted: its local and remote lifecycle runs passed, then reinit recovery stopped because 172.17.0.1 is not assigned to this Docker-host environment; the dedicated registrar-internal-init arm separately stopped at its required passwordless sudo -n prerequisite. CI's green Docker E2E jobs gate both privileged coverage paths.

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Reviewer Round 1]

Changes requested — two findings:

  • P2: The rendered recovery command leaves --agent-config unquoted in audit_store.rs. A valid config path containing spaces will not rerun; shell metacharacters also turn an operator-copyable root command into shell input. Quote the path with the existing POSIX-shell quoting approach and test a path with spaces/metacharacters.

  • P2: The new filesystem-mode infra up must write/replace staged artifacts (audit_store.rs), which requires root once init has created the staging directory. The E2E was updated accordingly, but the manual presents the new command without that prerequisite and still calls infra up unprivileged (operations.md, operations.md). Document the root-run requirement (and mirror it in Korean), or provide a clear early privilege refusal.

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Review Verdict Round 1: NOT_APPROVED]

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Author Round 1]

Review dispositions:

  • Fixed: quoted the rendered --agent-config recovery path with the existing POSIX-shell path quoter. Added coverage for spaces, ;, $(), quotes, and $HOME.
  • Fixed: documented the root requirement for filesystem-mode infra up in English and Korean operations guides, and corrected the mounted-reserve E2E documentation.

Pushed commit 091b0f5a to PR #948.

Validation: focused regression test, formatting, check.sh, test-core.sh, and docs build passed. The E2E matrix’s local/remote no-hosts lifecycles passed; it stopped at the known reinit host-address limitation (172.17.0.1 unavailable).

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Reviewer Round 2]

Round 1 findings are resolved: the recovery path is safely shell-quoted, and the filesystem-mode root requirement is documented in both manuals.

Changes requested:

  • P2: The unchanged localized error for a colon in audit_store_dir now gives the wrong reason. The new long-form Compose bind has a separate source: field, so Compose no longer splits that path on :; the code comment correctly explains that the restriction remains for the reader, systemd escaping, and rendered commands. Update both message catalogues to reflect that rationale while retaining the refusal. en.rs and ko.rs.

Verdict: not approved pending this documentation/message correction.

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Review Verdict Round 2: NOT_APPROVED]

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Author Round 2]

Fixed — updated English and Korean audit-store path-refusal messages to explain why colons remain unsupported under the long Compose bind form. Updated adjacent test documentation accordingly.

Pushed commit ac2b4853 to PR #948.

Validation: formatting, Clippy, rustdoc, docs build, full cargo test passed. Full preflight reached E2E coverage; its privileged matrix remains environment-gated as already documented in the PR.

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Reviewer Round 3]

[Reviewer Round 3]

Round 2’s localized colon-path rationale is correctly updated in both catalogues; no prior findings remain open.

Changes requested:

  • P2: The required end-to-end infra up refusal against an unmounted filesystem store is still not tested. run-registrar-internal-init-e2e.sh:741 tests create_host_path: false by invoking Compose directly, while the Rust test only exercises prepare_audit_store_for_infra_up without a Docker call. Neither would fail if the audit-store preparation regressed to run after Compose had started OpenBao. Add an E2E case that invokes bootroot infra up --agent-config ... on the deliberately unmounted store and asserts its nonzero provisioned, not activated refusal, with no OpenBao container running.

Verdict: NOT_APPROVED pending that coverage.

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Review Verdict Round 3: NOT_APPROVED]

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Author Round 3]

Fixed — added a registrar E2E case that runs bootroot infra up --agent-config against the deliberately unmounted filesystem store. It requires the provisioned, not activated refusal and verifies neither an OpenBao container nor <store>/openbao was created.

Pushed a5aab33f to PR #948.

Validation: syntax check and preflight quality checks passed. The focused E2E is locally blocked by unavailable Docker and passwordless sudo; CI will run it.

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Reviewer Round 4]

[Reviewer Round 4]

Approved.

The prior P2 is resolved: the new E2E invokes bootroot infra up --agent-config against the deliberately unmounted filesystem store, requires the provisioned, not activated refusal, and verifies that neither the OpenBao container nor the absent bind source was created (run-registrar-internal-init-e2e.sh).

The updated implementation continues to gate before Compose pull/up (infra.rs), retains the documented legacy-bind residual, and the PR linkage/test plan are appropriate. No unresolved or new findings.

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

[Review Verdict Round 4: APPROVED]

@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Suggested squash commit

Title

Refuse unmounted audit store bring-up

Body

Prevent `infra up` from starting OpenBao before the filesystem-backed
audit reserve has been verified, and make failed mounts fail visibly at
boot instead of allowing Docker to create an unprotected audit path.

Closes #924
Part of #773
Part of #775

@sehkone
sehkone force-pushed the sehkone/issue-924 branch from a5aab33 to fac34d4 Compare August 28, 2026 04:46
Prevent `infra up` from starting OpenBao before the filesystem-backed
audit reserve has been verified, and make failed mounts fail visibly at
boot instead of allowing Docker to create an unprotected audit path.

Closes #924
Part of #773
Part of #775
@sehkone
sehkone force-pushed the sehkone/issue-924 branch from fac34d4 to ee0f7c6 Compare August 28, 2026 04:56
@sehkone

sehkone commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main commit 529ef04 (Refuse registrar without audit mount). Resolved the English and Korean operations-guide conflicts by retaining the branch’s Compose bind behavior and main’s daemon-side unmounted-store refusal.

@sehkone
sehkone merged commit 70dcaf6 into main Aug 28, 2026
21 checks passed
@sehkone
sehkone deleted the sehkone/issue-924 branch August 28, 2026 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refuse the bring-up and the boot path on an unmounted audit store

1 participant