119 lines
5.4 KiB
Markdown
119 lines
5.4 KiB
Markdown
# 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.
|