From 819ccb155fea3cca09275cd91855490507d91b00 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 7 Sep 2026 18:51:14 +0000 Subject: [PATCH] docs: record Seriatim remediation findings --- docs/roadmap/remediation.md | 118 ++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 docs/roadmap/remediation.md diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..6bd1cd9 --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,118 @@ +# Feature 0001 Remediation Findings + +Status: open + +## Review Scope + +This review compared `ansible-role-seriatim` commits `ac35c1f` through +`efd4734` with the Feature 0001 roadmap and staged implementation plan. It also +ran the committed static and integration validation and performed targeted +state-corruption, permission-drift, and custom-path checks on the managed local +installation. + +The four planned implementation stages are structurally present and the +committed integration suite passes. The findings below are gaps that must be +addressed before Feature 0001 is marked complete or its patterns are copied to +other application roles. + +## Finding 1: Build-input state content is not part of convergence + +Severity: high + +`tasks/build.yml` checks whether `build-inputs.json` exists but never reads or +compares its content. The rebuild decision trusts `fingerprint.sha256` +independently. An empty, corrupted, stale, or replaced build-input document is +therefore accepted as converged when the fingerprint and binary checksum files +remain unchanged. + +This diverges from the plan's requirement that malformed or unreadable state +cause a rebuild and from the role's claim that readable inputs and their +fingerprint describe the installed result. A targeted test truncated +`build-inputs.json`; convergence returned `changed=0` and `failed=0`, while the +subsequent verification play correctly failed its input/fingerprint assertion. + +Required remediation: + +- Read the stored build-input document safely during convergence. +- Require its exact canonical content to match the desired build inputs and + require its SHA-256 digest to match the stored and desired fingerprint. +- Treat missing, unreadable, malformed, or inconsistent input state as a + rebuild condition rather than an unhandled failure. +- Extend `tests/integration` to corrupt or replace `build-inputs.json`, verify + that convergence repairs it, and verify that the following run is + idempotent. + +## Finding 2: Managed ownership and modes drift without repair + +Severity: medium + +The binary's checksum and version participate in the rebuild decision, but its +owner, group, and mode do not. Binary metadata is set only inside the rebuild +block. Build-state directory and file metadata are likewise set only after a +build. A content-identical installation can therefore remain in an unsafe or +incorrect metadata state indefinitely. + +A targeted test changed `/usr/local/bin/seriatim` from mode `0755` to `0777`. +Convergence returned `changed=0` and left mode `0777` in place. This violates +the target state requiring a root-owned, non-operator-writable binary. The same +design leaves drifted state-file ownership and permissions uncorrected. + +Required remediation: + +- Converge `seriatim_binary_path` to `root:root` mode `0755` whenever a valid + installed regular file exists, without forcing an unnecessary rebuild. +- Converge `seriatim_build_state_dir` to `root:root` mode `0755` and its three + managed files to `root:root` mode `0644` on every successful role run. +- Treat unexpected file types, including a directory at the binary path, as a + clear validation or repair condition rather than passing them into the build + copy operation. +- Add integration cases that drift binary and build-state metadata, require + one repairing run, and require an idempotent repeat. + +## Finding 3: A configurable binary path requires a pre-existing parent + +Severity: medium + +`seriatim_binary_path` is a documented public path override, but the role does +not create or validate its parent directory. `ansible.builtin.copy` consequently +fails after a successful build when the configured parent is absent. A +targeted run using `/tmp/seriatim-review-missing-parent/seriatim` failed with +`Destination directory ... does not exist`. + +Required remediation: + +- Inspect the configured binary parent before building. +- Fail clearly if an existing parent is not a directory. +- Create a missing parent as `root:root` mode `0755` without changing ownership + or mode on an already existing shared directory. +- Add an isolated integration case proving that a nondefault binary path with + a missing parent installs, validates, and converges idempotently. + +## Finding 4: Role metadata and privilege documentation are incomplete + +Severity: low + +Stage 1 required role source and issue information, but `meta/main.yml` omits +the supported `issue_tracker_url` field. The README also does not state plainly +that consumers must apply the role with privilege escalation, even though it +manages packages, snaps, root-owned source, and `/usr/local/bin`. + +Required remediation: + +- Add the role repository's Gitea issue tracker to `meta/main.yml`. +- Add role repository and issue links to the README where appropriate. +- State explicitly that the consuming play must use `become: true` or provide + equivalent root privileges. +- Re-run metadata/schema validation and ansible-lint. + +## Remediation Completion Criteria + +- All four findings are addressed in `ansible-role-seriatim`. +- Static validation and the expanded `tests/integration` suite pass. +- Targeted corruption, metadata-drift, and alternate-path cases converge to + the documented state and then report `changed=0` on an immediate repeat. +- The default local installation ends at Seriatim `v1.5.0`, source commit + `0fc92f364316c874d366b00cdc884a33e1900b25`, with a clean checkout and + consistent build state. +- The role and workspace repositories are clean, and remediation commits are + ready for review before Feature 0001 is marked complete.