138 lines
5.9 KiB
Markdown
138 lines
5.9 KiB
Markdown
# Feature 0001 Remediation Review
|
|
|
|
Status: resolved
|
|
|
|
## Finding 1: Synchronize Implementation Progress Tracking
|
|
|
|
Severity: low
|
|
|
|
The two implementation stages are marked complete in
|
|
`docs/roadmap/implementation.md`, with commits `325a5c7` and `718030e`, but the
|
|
same document still has status `proposed` and says that no stages have been
|
|
executed. `docs/status.md` likewise still describes plan review as the current
|
|
focus and Feature 0001 as `ready` rather than reflecting the completed
|
|
implementation and pending review outcome.
|
|
|
|
### Required Remediation
|
|
|
|
- Update the implementation plan status and progress notes to agree with its
|
|
two recorded stage completions.
|
|
- Update workspace status to reflect that implementation is complete and the
|
|
feature is awaiting review acceptance.
|
|
- Preserve the prior remediation history in Git rather than carrying its
|
|
superseded contents in the active remediation document.
|
|
|
|
### Validation
|
|
|
|
- The implementation plan's status, stage completion annotations, and progress
|
|
notes describe one consistent state.
|
|
- Workspace status names implementation review as the current activity and
|
|
uses the corresponding feature state.
|
|
- `git diff --check` passes.
|
|
|
|
## Finding 2: Remove Production Version Interrogation
|
|
|
|
Severity: medium
|
|
|
|
`ansible-role-seriatim/tasks/main.yml` runs the installed binary before a build
|
|
to detect unexpected version output and again after a build to validate the
|
|
result. Neither operation is part of the required production convergence
|
|
contract. The first adds unrequested drift detection, while the second turns a
|
|
role task into a test of the role's own build implementation.
|
|
|
|
### Required Remediation
|
|
|
|
- Remove the pre-build `--version` command and its registered result.
|
|
- Rebuild only when the Git checkout changed or the binary is absent.
|
|
- Retain the binary `ansible.builtin.stat`, because its result is independently
|
|
needed to decide whether the imperative build must run.
|
|
- Remove the post-build `--version` command, its registered build result, and
|
|
the associated lint suppression.
|
|
- Continue verifying the installed version in repository-local integration
|
|
tests.
|
|
|
|
### Validation
|
|
|
|
- The role contains no production invocation of `seriatim --version`.
|
|
- The build task has exactly the two required triggers: changed checkout or
|
|
absent binary.
|
|
- Missing-binary and changed-checkout integration cases each cause a build and
|
|
then converge to an unchanged repeat.
|
|
- The integration suite verifies the built binary's exact version output.
|
|
|
|
## Finding 3: Complete the Declarative Dependency and Privilege Contract
|
|
|
|
Severity: medium
|
|
|
|
The build command hard-codes `/snap/bin/go`, which makes dependency opt-out
|
|
unnecessarily dependent on the Snap installation path. The Git package task
|
|
also refreshes the APT cache without a validity interval, and tasks that modify
|
|
system-owned paths or install system dependencies currently rely on privilege
|
|
escalation supplied by the calling play.
|
|
|
|
### Required Remediation
|
|
|
|
- Add `seriatim_go_binary`, defaulting to `/snap/bin/go`, to the public defaults
|
|
and argument specification, and use it as the build command executable.
|
|
- Add `cache_valid_time: 3600` to the Git `ansible.builtin.apt` task.
|
|
- Set `become: true` directly on tasks that install dependencies or modify
|
|
root-owned application state. Do not add privilege escalation to the
|
|
platform assertion or binary inspection merely for uniformity.
|
|
- Update the README's variable table, dependency opt-out description, and
|
|
privilege behavior to match the implemented contract.
|
|
|
|
### Validation
|
|
|
|
- Dependency opt-out can use a caller-supplied Go executable.
|
|
- The APT task retains declarative cache management without refreshing a valid
|
|
cache.
|
|
- A play connecting as an ordinary sudo-capable user can apply the role without
|
|
play-level `become: true`.
|
|
- Defaults, argument specifications, tasks, and README agree on the seven
|
|
public variables.
|
|
|
|
## Finding 4: Align the Feature Record and Focused Tests
|
|
|
|
Severity: medium
|
|
|
|
The active feature roadmap and completed implementation plan still require
|
|
wrong-version drift detection, post-build validation as a production task, a
|
|
hard-coded Go executable, and a six-variable interface. The integration driver
|
|
also contains a wrong-version rebuild case that would preserve the superseded
|
|
contract.
|
|
|
|
### Required Remediation
|
|
|
|
- Revise the feature roadmap to record the two rebuild triggers, the
|
|
seven-variable interface, and the approximately eight-task target flow.
|
|
- Annotate or revise the completed implementation plan so it does not remain an
|
|
apparently authoritative description of the superseded target.
|
|
- Remove the wrong-version rebuild case from the integration driver.
|
|
- Retain focused coverage of default convergence and idempotence, a missing
|
|
binary, binary metadata convergence without a build, a changed selected
|
|
version, platform rejection, and final version output.
|
|
- Update the README to describe only the revised behavior.
|
|
|
|
### Validation
|
|
|
|
- Workspace roadmap, implementation record, role implementation, README, and
|
|
integration suite describe the same final architecture.
|
|
- No active document claims that unexpected version output triggers a rebuild
|
|
or that the production role validates a newly built executable by running
|
|
it.
|
|
- `tests/integration`, `bash -n tests/integration`, and `git diff --check` pass.
|
|
|
|
## Resolution
|
|
|
|
All four findings were resolved in `ansible-role-seriatim` commit `fb8dc41` on
|
|
2026-09-08. The role now uses an eight-task flow with two rebuild triggers, a
|
|
caller-selectable Go executable, bounded APT cache refreshes, and task-level
|
|
privilege escalation. Production tasks do not execute the installed
|
|
application; repository-local verification retains the version assertion. The
|
|
active roadmap, implementation record, status, README, interface, and focused
|
|
tests describe the same architecture.
|
|
|
|
Validation completed successfully with `tests/integration`,
|
|
`bash -n tests/integration`, and `git diff --check` in both changed
|
|
repositories.
|