150 lines
7.4 KiB
Markdown
150 lines
7.4 KiB
Markdown
# Feature 0011 Remediation Review
|
|
|
|
Status: resolved
|
|
|
|
Reviewed implementation commits:
|
|
|
|
- `292d866 Upgrade the Narratio role to version 2.1.0`
|
|
- `28a4d27 Add managed credentials to the D&D pipeline`
|
|
- `8ff0761 Document and test pipeline credential delivery`
|
|
|
|
## Review Summary
|
|
|
|
Feature 0011 implements the principal target architecture. The standalone
|
|
Narratio role now installs v2.1.0 and generically exposes its three native
|
|
Notarius controls. The pipeline pins that role revision, manages a restricted
|
|
conventional secrets directory, coordinates the Audita and Notarius credential
|
|
references, and enables ordinary Notarius checkpoint reuse without duplicating
|
|
the root-owned chunk-cache policy. Both complete integration entry points pass,
|
|
including native application validation and immediate clean repeats.
|
|
|
|
Two issues remain. First, the feature roadmap and implementation plan
|
|
strengthened the requested credential-delivery interface with deletion and
|
|
empty-value policies that were not required. The implementation consequently
|
|
contains an extra production task and substantial test and documentation
|
|
machinery for those invented semantics. Second, the pipeline README does not
|
|
explain the Scriptorium credential flow that the feature roadmap expressly
|
|
requires it to document.
|
|
|
|
## Finding 1: The Credential Mapping Adds Unrequested Lifecycle Semantics
|
|
|
|
Severity: moderate
|
|
|
|
The user requested an inventory-friendly way to deliver credential values to
|
|
Narratio's conventional secrets directory. The roadmap subsequently added two
|
|
stronger behaviors without an express requirement: null means delete an exact
|
|
file, and an empty string must be rejected by the role. Those choices were
|
|
then made mandatory by the implementation plan.
|
|
|
|
The resulting production implementation appears in
|
|
`ansible-role-dnd-pipeline/tasks/main.yml:10-53`. It validates the stronger
|
|
value policy, conditionally copies non-null entries, and uses a separate file
|
|
loop to delete null entries. The public contract and example are repeated in
|
|
`ansible-role-dnd-pipeline/README.md:275-291`, while
|
|
`ansible-role-dnd-pipeline/tests/integration:167-194` adds an empty-value
|
|
failure case and a multi-convergence deletion lifecycle.
|
|
|
|
The environment-variable-name validation is justified because caller input
|
|
becomes a destination path. The two value semantics are different: they add
|
|
credential lifecycle and application-validity guarantees that the role was
|
|
not asked to own. ConfigKit accepts an empty immediate credential file and
|
|
performs its own nonempty check only when a configured typed reference resolves
|
|
to that value. A simple delivery role should write the caller-supplied string
|
|
and trust the native application boundary. Credential deletion can be added
|
|
later if an operator-facing removal contract is expressly requested.
|
|
|
|
### Required Remediation
|
|
|
|
- Narrow `dnd_pipeline_secrets` to an empty-by-default mapping from valid
|
|
environment-variable names to string file contents.
|
|
- Retain complete key validation before path construction and retain
|
|
`no_log: true` for every task that evaluates or writes credential values.
|
|
- Validate that values are strings, but do not add a nonempty policy that is
|
|
stricter than the native bootstrap and typed-reference boundary.
|
|
- Converge every supplied entry directly with one `ansible.builtin.copy` loop.
|
|
Remove the null condition and the separate deletion task.
|
|
- Treat omitted keys as unmanaged. Do not invent a replacement deletion,
|
|
purge, directory-mirroring, or credential-validation mechanism.
|
|
- Remove the empty-value and explicit-null lifecycle cases from the integration
|
|
suite. Retain the valid-name boundary case, representative credential
|
|
delivery, redaction checks, authorized native loading, and immediate clean
|
|
repeat.
|
|
- Correct the Feature 0011 roadmap, the settled MVP text, and the pipeline
|
|
README so they describe the narrower interface and no longer present the
|
|
strengthened semantics as user decisions.
|
|
|
|
## Finding 2: Scriptorium Credential Delivery Is Not Documented for Consumers
|
|
|
|
Severity: low
|
|
|
|
The Feature 0011 roadmap requires documentation of all application credential
|
|
flows, including Scriptorium's existing profile-owned `api_key_env` selection.
|
|
`ansible-role-dnd-pipeline/README.md:275-291` explains the generic mapping and
|
|
the earlier Notarius section explains its typed reference, but the README never
|
|
tells an operator how a selected Scriptorium profile obtains its credential.
|
|
|
|
Production behavior is already correct: Narratio bootstraps the named file into
|
|
its environment and its existing Scriptorium adapter forwards the selected
|
|
profile credential. This is a documentation gap, not a request for another
|
|
variable, task, or integration test.
|
|
|
|
### Required Remediation
|
|
|
|
- Add a concise consumer-facing explanation that a Scriptorium profile remains
|
|
the owner of its `api_key_env` name.
|
|
- Explain that an operator can add a matching filename/value entry to
|
|
`dnd_pipeline_secrets`; Narratio's bootstrap supplies it and the existing
|
|
adapter forwards only the profile-selected credential.
|
|
- Do not add a suite-specific Scriptorium credential variable, inspect profile
|
|
files in Ansible, duplicate the application adapter, or add tests of
|
|
Narratio/Scriptorium internals.
|
|
|
|
## Resolution
|
|
|
|
Resolved in pipeline-role commit `de1b191`. The credential mapping now accepts
|
|
valid environment-variable names with string contents, validates the complete
|
|
mapping before path construction, and converges every supplied entry through
|
|
one secret-safe `ansible.builtin.copy` loop. The unrequested empty-value policy,
|
|
null deletion task, and associated lifecycle tests were removed. Omitted names
|
|
remain unmanaged.
|
|
|
|
The pipeline README now explains how a Scriptorium profile's application-owned
|
|
`api_key_env` name selects a matching `dnd_pipeline_secrets` entry without
|
|
adding another suite variable or duplicating adapter behavior. The durable
|
|
Feature 0011 roadmap and MVP policy were corrected to record the narrower
|
|
operator-facing contract.
|
|
|
|
## Required Validation
|
|
|
|
Remediation validation:
|
|
|
|
```text
|
|
cd ansible-role-dnd-pipeline && tests/integration
|
|
git diff --check
|
|
```
|
|
|
|
The result was successful syntax and lint validation; synthetic
|
|
credential delivery through the conventional directory; native Narratio
|
|
configuration loading for authorized identities; absence of synthetic values
|
|
from captured output; preservation of Notarius resume and root-owned `auto`
|
|
chunk caching; and immediate repeats with `changed=0` and `failed=0`.
|
|
|
|
## Review Validation Evidence
|
|
|
|
The review inspected the exact implementation diffs and confirmed that the
|
|
Narratio dependency pin resolves to the reviewed `292d866` revision. The
|
|
standalone Narratio `tests/integration` entry point passed with v2.1.0 native
|
|
configuration validation and clean repeats. The complete pipeline
|
|
`tests/integration` entry point also passed, including isolated dependency
|
|
installation, syntax checking, production-profile lint, authorized and denied
|
|
native loading, redaction checks, profile and version selections, and clean
|
|
repeats. No live model, transcription, storage, or publishing endpoint was
|
|
contacted.
|
|
|
|
After remediation, the complete pipeline integration entry point passed again.
|
|
It exercised isolated dependency installation, the invalid-name boundary,
|
|
synthetic credential delivery and redaction, authorized native loading,
|
|
Notarius resume and root-owned `auto` chunk caching, the retained profile and
|
|
version selections, and immediate clean repeats. `git diff --check` also
|
|
passed before the remediation commit.
|