Complete the shared pipeline filesystem feature
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# Feature 0009 Remediation Review
|
||||
|
||||
Status: open
|
||||
Status: resolved
|
||||
|
||||
Reviewed implementation commits:
|
||||
|
||||
@@ -8,164 +8,83 @@ Reviewed implementation commits:
|
||||
- `e9ae2dc Make Notarius configuration ownership configurable`
|
||||
- `8b4c86e Make Scriptorium configuration ownership configurable`
|
||||
- `eeb1105 Make Narratio configuration ownership configurable`
|
||||
- `901ae35 Apply shared configuration ownership to the D&D pipeline`
|
||||
- `dddc5ee Manage shared pipeline access with ACLs`
|
||||
- `f53da82 Test shared pipeline access across users`
|
||||
- `608d5be Document the shared pipeline execution model`
|
||||
|
||||
## Review Summary
|
||||
|
||||
The four completed standalone-role stages are concise, idiomatic, and aligned
|
||||
with Feature 0009. Each role exposes owner, group, configuration-directory
|
||||
mode, and configuration-file mode variables; applies them directly through the
|
||||
ordinary `file` and `template` tasks; preserves its standalone defaults; and
|
||||
documents the interface. No redundant inspection, corrective convergence,
|
||||
private state, or unrelated application behavior was introduced. All four
|
||||
repository-local integration suites passed independently during review.
|
||||
The production architecture and behavioral coverage implement Feature 0009 as
|
||||
planned. The four standalone roles expose generic configuration ownership and
|
||||
mode controls, and the pipeline selects its opinionated service-owned,
|
||||
world-readable policy through those interfaces. Narratio establishes the
|
||||
runtime identity first. The pipeline then installs ACL support and directly
|
||||
converges access and default ACLs on the three existing shared roots. Focused
|
||||
integration coverage exercises actual cross-identity configuration access and
|
||||
bidirectional writes under a restrictive umask while preserving immediate clean
|
||||
convergence.
|
||||
|
||||
Feature 0009 as a whole is not implemented. The pipeline repository remains at
|
||||
pre-feature commit `3e253d7`; Stages 5 through 8 have not been executed. The
|
||||
publication blocker encountered after Stage 4 has now been cleared: all four
|
||||
standalone commits are available from their canonical origins. The following
|
||||
findings therefore describe the remaining target architecture rather than
|
||||
defects in the completed standalone commits.
|
||||
No production code or test-suite remediation is required. One small consumer
|
||||
documentation inconsistency remains.
|
||||
|
||||
## Finding 1: The Pipeline Does Not Select the Shared Configuration Policy
|
||||
## Finding 1: Operator Support Is Documented Inconsistently
|
||||
|
||||
Severity: high
|
||||
Severity: low
|
||||
|
||||
`ansible-role-dnd-pipeline/defaults/main.yml` and
|
||||
`meta/argument_specs.yml` do not define the four suite-level runtime identity
|
||||
and configuration-mode variables. `meta/main.yml:19-98` still runs Narratio
|
||||
last and does not pass the new configuration ownership interfaces to Audita,
|
||||
Notarius, Scriptorium, or Narratio. `requirements.yml:7-22` still pins the four
|
||||
pre-feature role commits.
|
||||
`ansible-role-dnd-pipeline/README.md:201-217` first lists "operator access" as
|
||||
deferred, then correctly states that invocation by an existing member of the
|
||||
selected runtime group is supported. The likely deferred concern is operator
|
||||
selection and group-membership management, not operator access itself.
|
||||
|
||||
Consequently, applying the current pipeline role retains standalone ownership:
|
||||
Audita, Notarius, and Scriptorium configuration remains `root:root`, while
|
||||
Narratio configuration remains `root:narratio` with its standalone restrictive
|
||||
modes. The target `narratio:narratio`, `0755` directory, and `0644` file suite
|
||||
policy is not selected. Narratio also does not establish the service identity
|
||||
before the other configuration-bearing dependencies run.
|
||||
The adjacent YAML fragment defines an otherwise unexplained
|
||||
`operator_group_members` variable but shows no consumer task or role that uses
|
||||
it. As written, copying the example does not manage any membership and may lead
|
||||
a reader to believe that the pipeline role consumes that variable.
|
||||
|
||||
### Required Remediation
|
||||
|
||||
- Use the four published, immutable standalone-role commits recorded above;
|
||||
do not replace their commit pins with branches.
|
||||
- Execute Stage 5 of `docs/roadmap/implementation.md` in full.
|
||||
- Add the four suite defaults and argument-spec entries, run Narratio first,
|
||||
pass the selected identity and modes through every generic role interface,
|
||||
update the four dependency pins, and apply the same values directly to the
|
||||
pipeline-owned Narratio fragments.
|
||||
- Retain direct module convergence without any post-dependency permission
|
||||
repair.
|
||||
- Replace "operator access" in the deferred list with the precise deferred
|
||||
behavior: operator selection and group-membership management.
|
||||
- Replace the inert variable fragment with a concise, complete consumer-side
|
||||
example that actually adds a selected account to the runtime group, or omit
|
||||
the example and state the prerequisite directly.
|
||||
- Keep human account and membership management outside the production pipeline
|
||||
role.
|
||||
|
||||
## Finding 2: Shared Writable-Root ACL Convergence Is Absent
|
||||
## Resolution
|
||||
|
||||
Severity: high
|
||||
|
||||
`ansible-role-dnd-pipeline/collections/requirements.yml:1-4` contains only
|
||||
`community.general`. `tasks/main.yml:1-65` neither installs Ubuntu's `acl`
|
||||
package nor uses `ansible.posix.acl`. The three Narratio writable roots still
|
||||
receive mode `2775` from the standalone role, but the suite does not establish
|
||||
the access and default ACLs required to preserve group write and world
|
||||
read/traverse behavior across invocation identities with different umasks.
|
||||
|
||||
### Required Remediation
|
||||
|
||||
- Execute Stage 6 of the implementation plan in full.
|
||||
- Pin `ansible.posix` 2.2.0 alongside the existing collection dependency.
|
||||
- Install `acl` with one ordinary privileged `ansible.builtin.apt` task.
|
||||
- Apply the selected access and default ACL entries to exactly
|
||||
`/var/lib/narratio`, `/var/spool/narratio`, and `/var/cache/narratio` with
|
||||
direct, non-recursive `ansible.posix.acl` convergence.
|
||||
- Do not add filesystem probes, `getfacl` production commands, recursive
|
||||
migration, handler flow, or supplemental permission repair.
|
||||
|
||||
## Finding 3: The Shared Execution Contract Has No Behavioral Coverage
|
||||
|
||||
Severity: medium
|
||||
|
||||
`ansible-role-dnd-pipeline/tests/integration:64-133` exercises the established
|
||||
pipeline only as root and the `narratio` service account. It does not use a
|
||||
representative existing member of the shared group, exchange writable-state
|
||||
content between invocation identities, exercise inheritance under a
|
||||
restrictive ordinary umask, or observe the deliberate world-readable boundary.
|
||||
|
||||
The missing tests are important because the group-sharing outcome is suite
|
||||
behavior authored by this feature, not a request to re-test an individual
|
||||
Ansible module. The tests should exercise actual access rather than assert
|
||||
mode values or parse ACL metadata.
|
||||
|
||||
### Required Remediation
|
||||
|
||||
- Execute Stage 7 of the implementation plan after Findings 1 and 2 are
|
||||
resolved.
|
||||
- Use one uniquely named, test-owned operator account as an integration
|
||||
fixture and clean up only that account and its explicitly named content.
|
||||
- Validate one representative Narratio operation as that group member and one
|
||||
bidirectional content exchange with the service account.
|
||||
- Observe one representative world-readable configuration or artifact as an
|
||||
account outside the group.
|
||||
- Preserve immediate clean repeated convergence and avoid an exhaustive
|
||||
identity, path, mode, or ACL matrix.
|
||||
|
||||
## Finding 4: Pipeline Consumer Documentation Is Stale
|
||||
|
||||
Severity: medium
|
||||
|
||||
`ansible-role-dnd-pipeline/README.md:185` still lists operator access among
|
||||
deferred concerns. It does not document the new suite variables,
|
||||
Narratio-first dependency order, service-owned and world-readable
|
||||
configuration, shared setgid roots, inherited ACL behavior,
|
||||
existing-group-member invocation, or the restricted future `secrets/`
|
||||
boundary.
|
||||
|
||||
### Required Remediation
|
||||
|
||||
- Execute Stage 8 of the implementation plan after the production behavior and
|
||||
tests are complete.
|
||||
- Clearly distinguish generic standalone defaults from the pipeline's
|
||||
opinionated overrides.
|
||||
- Document setgid terminology, the three shared roots, supported invocation
|
||||
identities, the fact that production does not manage human membership, and
|
||||
the deliberate non-secret boundary.
|
||||
- Do not promise the deferred secrets implementation, recursive migration, or
|
||||
Feature 0010 Notarius behavior.
|
||||
Resolved in pipeline-role commit `b15fe8d`. The deferred-work summary now
|
||||
names operator selection and group-membership management rather than operator
|
||||
access. The shared-filesystem section retains existing-group-member invocation
|
||||
as supported behavior and provides a complete consumer-owned
|
||||
`ansible.builtin.user` example that adds a selected account to the runtime
|
||||
group. No production role behavior changed.
|
||||
|
||||
## Required Validation
|
||||
|
||||
After remediation, run the complete integration entry point in each changed
|
||||
role:
|
||||
|
||||
```text
|
||||
cd ansible-role-audita && tests/integration
|
||||
cd ../ansible-role-notarius && tests/integration
|
||||
cd ../ansible-role-scriptorium && tests/integration
|
||||
cd ../ansible-role-narratio && tests/integration
|
||||
cd ../ansible-role-dnd-pipeline && tests/integration
|
||||
```
|
||||
|
||||
Then run from the workspace root:
|
||||
After the documentation correction, run:
|
||||
|
||||
```text
|
||||
cd ansible-role-dnd-pipeline && tests/integration
|
||||
git diff --check
|
||||
./scripts/status
|
||||
```
|
||||
|
||||
The expected result is that all five role suites pass syntax and lint checks,
|
||||
native application checks succeed, the shared cross-identity contract is
|
||||
observed, every immediate repeat reports `changed=0` and `failed=0`, all eight
|
||||
implementation-stage commits are identified, and every repository is clean.
|
||||
The expected result remains successful native configuration checks,
|
||||
representative cross-identity access, and immediate repeats with `changed=0`
|
||||
and `failed=0`.
|
||||
|
||||
## Review Validation Evidence
|
||||
|
||||
During this review, the complete `tests/integration` entry points passed in
|
||||
`ansible-role-audita`, `ansible-role-notarius`,
|
||||
`ansible-role-scriptorium`, and `ansible-role-narratio`. Each suite installed
|
||||
its pinned collection dependencies, passed syntax checking and
|
||||
production-profile lint, exercised its native application behavior, and
|
||||
reported clean immediate repeats. `git diff --check` passed and all
|
||||
implementation repositories remained clean after testing.
|
||||
The independent review inspected the complete diffs for all eight stage
|
||||
commits and confirmed that every dependency pin resolves to the intended
|
||||
standalone-role commit. `git diff --check` passed across all five changed role
|
||||
repositories. The complete pipeline `tests/integration` entry point passed its
|
||||
isolated dependency installation, syntax check, production-profile lint,
|
||||
native application checks, cross-identity access exercise, configurable
|
||||
profile and version cases, and immediate clean repeats. All repositories were
|
||||
clean before this review document was revised.
|
||||
|
||||
The pipeline suite was not presented as Feature 0009 validation evidence
|
||||
because the pipeline repository contains no Feature 0009 implementation
|
||||
commits. Its current test suite exercises the prior Feature 0008 architecture.
|
||||
After review, each standalone commit was pushed to its canonical `origin/main`
|
||||
branch and the local and remote-tracking revisions were confirmed identical.
|
||||
After remediation, the complete pipeline integration entry point and
|
||||
`git diff --check` passed again. The test fixture was removed and the baseline
|
||||
Seriatim version and pipeline configuration were restored during cleanup.
|
||||
|
||||
Reference in New Issue
Block a user