Document configuration and composition audit findings
This commit is contained in:
@@ -103,7 +103,7 @@ These are prioritization signals, not findings:
|
||||
| 3: paths and filesystem | complete | Canonical local/remote paths, every artifact source family, filesystem mutations, directory promotion, cleanup confinement, permissions, and lock mechanics reviewed. Focused normal and race commands passed. Confirmed `COR-002`, `COR-003`, `RSK-002`, `RSK-003`, and `RSK-004`; added `DUP-002` and `TST-003`, and refined `DUP-001`. |
|
||||
| 4: publish and cleanup | complete | Publish prerequisites/source families, deterministic upload order, every partial remote outcome, retry semantics, effective locks, status/restore interpretation, and automatic/manual cleanup gates reviewed. Focused stage/app/artifacts/storage tests passed. Confirmed `COR-004` through `COR-007` and `RSK-005`; added `ARC-003`, `COM-002`, and `TST-004`. |
|
||||
| 5: restore and previous state | complete | Restore discovery/planning/execution/reporting, remote-current identity and scope, every local failure boundary, audio cache/spool identity, previous-cache planning/consumption, and status/validate policy reviewed. Focused app/previouscache/audio/artifacts/storage tests passed. Confirmed `COR-008` through `COR-011`, `RSK-006` through `RSK-008`, and `EFF-001`; added `DUP-003` and `TST-005`. |
|
||||
| 6: configuration and composition | not_started | Assigned configuration, CLI composition, and process-boundary areas below. |
|
||||
| 6: configuration and composition | complete | Discovery/precedence, strict loading, defaults/normalization, templates, cross-field validation, CLI selection, filesystem secrets, conditional adapter composition, and maintained examples reviewed. Exact focused tests and repository vet passed. Confirmed `COR-012` through `COR-015`, `RSK-009`, and `RSK-010`; added `ARC-004`, `DUP-004`, and `TST-006`. |
|
||||
| 7: adapters and shared support | not_started | Assigned external-boundary and cancellation areas below. |
|
||||
| 8: ordinary stages | not_started | Assigned prepare/transcript behavior and disabled-outcome questions below. |
|
||||
| 9: extraction | not_started | Assigned extraction promotion, provenance, and resume scenario below. |
|
||||
@@ -119,9 +119,9 @@ has been mapped but not behaviorally audited.
|
||||
|
||||
| Inspection area | Canonical implementation owner | Primary audit stage | Status |
|
||||
| --- | --- | --- | --- |
|
||||
| Process and application boundary | `cmd/narratio`, `internal/app` | 6 (runner lifecycle portions in 2; publish/restore portions in 4-5) | assigned |
|
||||
| Process and application boundary | `cmd/narratio`, `internal/app` | 6 (runner lifecycle portions in 2; publish/restore portions in 4-5) | reviewed |
|
||||
| Stage registry and runner | `internal/stage`, `internal/app` | 2 | reviewed |
|
||||
| Configuration | `internal/config` | 6 | assigned |
|
||||
| Configuration | `internal/config` | 6 | reviewed |
|
||||
| Prepare and audio | `internal/stage`, `internal/audio`, `internal/previouscache` | 8 | assigned |
|
||||
| Transcript stages | `internal/stage` plus tool adapters | 8 | assigned |
|
||||
| Extraction | `internal/stage`, Notarius adapter, `internal/fileops` | 9 | assigned |
|
||||
@@ -759,6 +759,102 @@ mutation, custom publish destination, optional operator-readiness, and foreign
|
||||
absolute-path cases above. `TST-005` assigns one stable behavior test per root
|
||||
invariant to Stage 12.
|
||||
|
||||
## Configuration And Composition Conclusions
|
||||
|
||||
### Discovery, precedence, defaults, and validation order
|
||||
|
||||
The process entry point delegates directly to `app.Execute`, which owns command
|
||||
parsing, exit classification, output streams, and dispatch. Pipeline selection
|
||||
uses an explicit path first and otherwise the first existing system default.
|
||||
Campaign selection rejects simultaneous ID and file selectors, loads an
|
||||
explicit file or registry/default ID, and checks that the selected ID agrees
|
||||
with the loaded campaign. Session selection uses an explicit path, then the
|
||||
first local default, then the configured campaign/session remote key when a
|
||||
session ID is available. Session stable inputs override campaign values; their
|
||||
resolved value retains the owning config path and source kind.
|
||||
|
||||
Pipeline, campaign, rendered-template, and session inputs all use the same
|
||||
known-field YAML decoder. Defaults are applied before resolution and validation.
|
||||
Pointer booleans and integers distinguish omission from explicit false/zero;
|
||||
an explicitly empty module list remains empty. Ordinary empty scalar and
|
||||
publish-output values receive documented defaults, while `normalize.output_path`
|
||||
tracks YAML presence so an explicit empty value is rejected. Enabled Notarius
|
||||
paths are resolved relative to the pipeline file. A concrete session template
|
||||
is rendered from a narrow variable set, rejected for missing/unknown/unused
|
||||
values, decoded through the ordinary session loader, resolved, and fully
|
||||
validated before replacement.
|
||||
|
||||
The final order is pipeline load/defaults, campaign load, session
|
||||
load/selection, stable-input resolution, then pipeline, campaign, session, and
|
||||
cross-config validation. The remote-session fallback necessarily builds enough
|
||||
pipeline/campaign/storage state to fetch the session before final validation;
|
||||
secrets are loaded before constructing that object store. This ordering is
|
||||
otherwise coherent, but the shared decoder's trailing-document check is
|
||||
incorrect under `COR-012`.
|
||||
|
||||
### Operator field-to-consumer trace
|
||||
|
||||
| Operator field family | Default/normalization and validation | Runtime consumer and conclusion |
|
||||
| --- | --- | --- |
|
||||
| Stage enablement/order, concurrency, locks, workspace/cache/campaign/spool roots | Canonical stage order is fixed; enabled stages, positive global concurrency, lock settings, and non-empty principal roots are validated. Relative configured roots remain supported. | Planning selects enabled/requested stages; the runner derives layout, local locks, artifact/manifest stores, and worker bounds from the resolved config. No unconsumed execution selector was found. |
|
||||
| Campaign/session IDs, stable inputs, audio source, and previous session | Resolution enforces campaign consistency and stable-input precedence; validation requires one audio mode and required campaign/session values, with S3 bucket cross-checks. | Selection, layout, audio materialization, publish keys, and previous-cache planning consume these values. Unsafe ID segment syntax remains `COR-002`; the previous-session CLI expectation is incomplete under `COR-015`. |
|
||||
| WhisperX, Seriatim, Audita, and Scriptorium command/protocol settings | Defaults fill endpoints, commands, retry/concurrency/tuning, timeouts, and artifact contracts; enums, safe paths, environment-variable names, dependencies, and cycles are checked. | Adapter constructors and transcript/analyze/trim/render stages consume the settings. Parseable non-positive durations can pass configuration but fail at composition or stage execution under `COR-013`. |
|
||||
| Notarius extraction and lane settings | Disabled configurations stay lightweight; enabled configurations require command, config/work paths, declared inputs/outputs, unique lanes, timeout, and positive concurrency. Relative paths are anchored to the pipeline file. | The subprocess runner is constructed only when extraction is selected and Notarius is enabled. Extraction behavior remains assigned to Stage 9; no unconditional external work was found here. |
|
||||
| Analyze artifact source/destination, previous requirements, trim/render bounds | Source/destination identities, normalized relative paths, uniqueness, dependency existence, cycles, and bounds syntax are validated. | Artifact resolution and Scriptorium/Seriatim stages consume the values. Deeper dependency semantics remain assigned to Stage 10 rather than being inferred from configuration shape. |
|
||||
| Publish outputs/backend, S3 storage, cleanup, and remote locks | Publish sources/destinations and S3/env/path prerequisites are cross-checked; cleanup and lock defaults are explicit. | Publish, storage construction, remote locking, restore, and cleanup consume these fields. The storage backend selector itself is not validated and is not authoritative under `COR-014`. |
|
||||
| Filesystem secrets directory and credential variable names | The directory is optional; relative values intentionally use process working directory. Entry names and configured env-var names use the environment-name grammar. Existing process values win. | Secrets are installed into the process environment before adapter/store construction; values are not copied into config, metadata, reports, or logs. Entry-type trust is unsafe under `RSK-010`. |
|
||||
| Notification backend, recipient, and timeout | Fields are accepted and the timeout is syntax-checked. | Production composition always injects the no-op sender, so these operator-facing settings currently have no behavioral consumer. `ARC-004` assigns the boundary decision to Stage 7. |
|
||||
|
||||
Validation is strong for enumerations, artifact paths, cross-stage dependencies,
|
||||
unique destinations, session/campaign consistency, environment names, and
|
||||
positive concurrency/tuning. The deliberate empty-value/default behavior in
|
||||
the maintained examples agrees with `docs/config.md`. Four maintained pipeline
|
||||
examples are loaded with representative local or S3 sessions by the config
|
||||
suite, and extraction contract tests preserve their published Notarius shapes.
|
||||
Examples contain only fictional endpoints and credential variable names, not
|
||||
secret values. The material documentation drift is the notification block:
|
||||
the public annotated example presents settings as optional configuration while
|
||||
the internal overview alone explains that notification is a placeholder/no-op.
|
||||
|
||||
### Secrets, conditional composition, and lifecycle
|
||||
|
||||
The filesystem secret loader reads directory entries in deterministic name
|
||||
order, skips directories and invalid environment names, trims only trailing
|
||||
line endings, preserves existing environment values, and reports only the
|
||||
directory and counts. Error and success messages name fields/files but never
|
||||
include secret content. S3 and Audita resolve values only at their adapter
|
||||
boundaries. No config serialization, manifest, report, stage metadata, or log
|
||||
path retaining a raw secret value was found.
|
||||
|
||||
The runner supplies lightweight WhisperX, Seriatim, Audita, and Scriptorium
|
||||
wrappers when callers do not inject them. Those constructors do not connect to
|
||||
external services or start subprocesses. Notarius construction is conditional
|
||||
on selected extraction, and object-store/remote-lock construction is
|
||||
conditional on selected behavior that needs remote state. These collaborators
|
||||
own no closeable process-level resource: HTTP clients and AWS clients are
|
||||
reused value wrappers and subprocesses are owned per invocation. Thus there is
|
||||
no adapter shutdown leak at the composition boundary. Loading secrets again in
|
||||
the object-store helper is redundant but bounded and preserves ordering; it is
|
||||
not a standalone efficiency finding.
|
||||
|
||||
Successful remote session discovery is the exception to otherwise explicit
|
||||
temporary-file ownership. It downloads `session.yml` to a system temporary
|
||||
file and retains that path in resolved provenance without any success cleanup.
|
||||
Full runs later copy the file into canonical inputs but still leave the private
|
||||
temporary copy; read-only and single-stage commands leak it directly. This is
|
||||
recorded as `RSK-009`.
|
||||
|
||||
Tests can inject an `Env` and production supplies defaults only for nil
|
||||
collaborators. This is an effective no-live-credential seam, and focused tests
|
||||
cover enabled/disabled composition without network access. The seam can,
|
||||
however, retain an injected `Env.Config` different from the separately supplied
|
||||
resolved config, splitting layout/manifest identity from secrets, adapters,
|
||||
locks, and stage behavior. Production never exposes that combination, so
|
||||
`TST-006` assigns a guard/fixture decision to Stage 12 rather than treating it
|
||||
as a production defect. Mutable package-level constructor seams and small
|
||||
single-stage command wrappers are likewise test/dispatch mechanics, not new
|
||||
architectural findings.
|
||||
|
||||
## Confirmed Findings
|
||||
|
||||
### `COR-001`: session identity initialization preserves stale invocation paths and accepts conflicting identity
|
||||
@@ -830,14 +926,15 @@ invariant to Stage 12.
|
||||
config/artifacts change. Define one strict opaque-segment contract, validate
|
||||
identities before layout/key construction, and make canonical helpers fail
|
||||
closed rather than return an escaped path. Decide compatibility for existing
|
||||
non-segment IDs explicitly in Stage 6.
|
||||
non-segment IDs explicitly; Stage 6 found only safe segment-style IDs in
|
||||
maintained examples and documentation, but deployed configuration is unknown.
|
||||
- Test changes: table-test separator, traversal, absolute/drive, dot, and mixed-
|
||||
slash identities at the configuration boundary; add artifacts tests proving
|
||||
no local directory or S3 key can escape its expected namespace. Add a direct
|
||||
unsafe-relative test for the exported previous-artifact constructor if it
|
||||
remains public.
|
||||
- Dependencies: Stage 6 owns configuration compatibility and validation.
|
||||
`COR-001` separately owns conflicts among otherwise valid identities;
|
||||
- Dependencies: Stage 6 confirmed that no configuration validator closes this
|
||||
boundary. `COR-001` separately owns conflicts among otherwise valid identities;
|
||||
`COR-003` owns symlink-based escape after lexical identities are safe.
|
||||
|
||||
### `COR-003`: filesystem mutation confinement follows symlinked destination ancestors
|
||||
@@ -1166,8 +1263,122 @@ invariant to Stage 12.
|
||||
output, absent mapping in an old manifest, and exact source mapping. Existing
|
||||
default-path/fallback tests remain as compatibility cases.
|
||||
- Dependencies: this mapping is also required for the committed restore scope
|
||||
in `COR-008`. Stage 6 owns configuration compatibility; Stage 10 owns later
|
||||
artifact-consumer semantics, not remote identity selection.
|
||||
in `COR-008`. Stage 6 confirmed that custom destinations are supported and
|
||||
validated, so remediation must preserve them; Stage 10 owns later artifact-
|
||||
consumer semantics, not remote identity selection.
|
||||
|
||||
### `COR-012`: strict YAML loading silently ignores a valid trailing document
|
||||
|
||||
- Category: confirmed correctness defect.
|
||||
- Locations/invariant: `internal/config/load.go` in
|
||||
`decodeStrictYAMLFromReader`, shared by pipeline, campaign, and session
|
||||
loaders. A strict configuration file must contain exactly one known-field
|
||||
document; content after it must not be silently discarded.
|
||||
- Evidence: after decoding the requested value, the loader decodes once into
|
||||
`extra` and returns an error only when that second decode returns a non-EOF
|
||||
error. A valid second document returns nil, so it is accepted and ignored.
|
||||
Existing strict-decode tests cover unknown fields but no multi-document input.
|
||||
- Realistic scenario: an operator or generated deployment file appends an
|
||||
override document with a different storage bucket, timeout, or stage setting.
|
||||
Validation succeeds against only the first document and the command runs with
|
||||
behavior different from the complete visible file.
|
||||
- Impact/likelihood/confidence: high operator-integrity impact; low-to-moderate
|
||||
likelihood because multi-document YAML is common in generated configuration;
|
||||
high confidence from the decoder control flow.
|
||||
- Estimated remediation scope and owner: small config-loader correction. Require
|
||||
the second decode to return EOF and reject any decoded value, including null,
|
||||
with a contextual trailing-document error.
|
||||
- Test changes: table-test pipeline, campaign, byte-session, and rendered
|
||||
template paths with a valid second mapping, null/empty separators, malformed
|
||||
trailing YAML, unknown fields, and ordinary single-document EOF.
|
||||
- Dependencies: none. Apply before reasoning about future configuration
|
||||
migration formats so compatibility is explicit rather than accidental.
|
||||
|
||||
### `COR-013`: configuration accepts non-positive durations that runtime consumers reject
|
||||
|
||||
- Category: confirmed correctness defect.
|
||||
- Locations/invariant: `internal/config/validate.go` in `validateDuration` and
|
||||
its WhisperX, Seriatim, Audita, Scriptorium, trim/render-bounds, and related
|
||||
callers; corresponding adapter and stage duration parsing. A configuration
|
||||
accepted by validation should satisfy constructor/stage preconditions.
|
||||
- Evidence: the shared validator trims and parses duration syntax but imposes no
|
||||
sign constraint. Zero or negative request timeouts therefore pass config
|
||||
validation, while WhisperX, Seriatim, Audita, and Scriptorium execution reject
|
||||
non-positive timeouts. A negative WhisperX retry delay similarly passes config
|
||||
validation and is rejected by the HTTP client constructor. Notarius uses a
|
||||
separate positive check and does not have this defect.
|
||||
- Realistic scenario: `narratio session validate` or plan reports a deployment
|
||||
valid with `whisperx.timeout: 0s`; the subsequent run fails before useful work
|
||||
when composition rejects the same value. Scriptorium artifact timeouts can
|
||||
fail later only when their stage is selected.
|
||||
- Impact/likelihood/confidence: medium correctness/operability impact; moderate
|
||||
typo or generated-config likelihood; high confidence from paired validation
|
||||
and consumer checks.
|
||||
- Estimated remediation scope and owner: small config-policy correction. Give
|
||||
timeout and delay fields explicit sign rules, share helpers only where their
|
||||
contracts truly match, and keep empty artifact timeout as documented fallback.
|
||||
- Test changes: table-test zero and negative values for every consumed duration,
|
||||
positive/subsecond values, empty optional artifact timeout, and a focused
|
||||
assertion that validated config reaches constructors without sign errors.
|
||||
- Dependencies: Stage 7 may refine adapter error contracts, but config should
|
||||
reject values that no current consumer accepts independently of that work.
|
||||
|
||||
### `COR-014`: the storage backend selector is neither validated nor authoritative
|
||||
|
||||
- Category: confirmed correctness defect.
|
||||
- Locations/invariant: `internal/config/validate.go` in `validateStorage` and
|
||||
cross-config S3 checks; `internal/adapters/storage/factory.go` in
|
||||
`NewObjectStoreFromConfig`. An operator-selected backend must be recognized
|
||||
and must determine the adapter that is constructed.
|
||||
- Evidence: storage validation checks S3 path, endpoint, and credential-name
|
||||
fields but never checks `storage.backend`. The factory selects S3 for backend
|
||||
`s3`, but also selects it for every other spelling whenever the defaulted S3
|
||||
block has a bucket. An unknown backend with a bucket silently becomes S3; an
|
||||
unknown backend without one can pass config validation and fail later only
|
||||
when a command needs remote storage.
|
||||
- Realistic scenario: `backend: s33` with a production bucket validates and
|
||||
uploads to S3 despite the operator's invalid selector. The same typo in a
|
||||
locally exercised config may stay latent until remote session discovery,
|
||||
publish, restore, or status constructs the store.
|
||||
- Impact/likelihood/confidence: high boundary-selection impact; moderate typo or
|
||||
migration likelihood; high confidence from validator and factory branches.
|
||||
- Estimated remediation scope and owner: small config/factory correction. Define
|
||||
the supported backend enum and disabled/local meaning, validate it before
|
||||
cross-field checks, and make construction switch only on that normalized
|
||||
value. Do not infer backend from populated provider fields.
|
||||
- Test changes: cover exact/case policy for supported values, unknown and empty
|
||||
selectors with and without bucket, commands that need/do not need storage,
|
||||
and factory non-invocation after invalid config.
|
||||
- Dependencies: Stage 7 owns provider adapter behavior, not the operator-facing
|
||||
selection policy established here.
|
||||
|
||||
### `COR-015`: the previous-session expectation flag is ignored when the session omits the field
|
||||
|
||||
- Category: confirmed correctness defect.
|
||||
- Locations/invariant: command flag construction and `internal/app/config_loader.go`
|
||||
session options; `internal/config/load.go` in `LoadSessionBytesWithOptions`.
|
||||
An option described and typed as an expected previous-session identifier must
|
||||
either establish that identity or reject a session that does not contain it.
|
||||
- Evidence: the loader rejects a mismatch only when both the expected option and
|
||||
decoded `previous_session_id` are non-empty. If the session omits the optional
|
||||
field, a supplied `--previous-session-id` neither fills it nor fails loading,
|
||||
so planning and execution behave exactly as if the flag were absent. Tests
|
||||
cover a non-empty mismatch but not omission.
|
||||
- Realistic scenario: an operator validates or runs session B while pinning
|
||||
previous session A at the command line. A session file that accidentally
|
||||
drops `previous_session_id` passes the expectation and runs without the
|
||||
intended previous-artifact context.
|
||||
- Impact/likelihood/confidence: medium cross-session behavior impact; moderate
|
||||
likelihood during generated/session-template edits; high confidence from the
|
||||
option contract and conditional comparison.
|
||||
- Estimated remediation scope and owner: small CLI/config-selection decision.
|
||||
Treat the flag as a strict expectation and reject omission, or explicitly
|
||||
define it as an override and populate before resolution; keep the chosen
|
||||
semantics consistent across local and remote session paths and documentation.
|
||||
- Test changes: cover absent/equal/different file values for local, remote, and
|
||||
rendered sessions, plus no-flag optional behavior.
|
||||
- Dependencies: `COR-011` owns remote artifact source mapping after an identity
|
||||
is selected; this finding owns selection of that identity.
|
||||
|
||||
### `RSK-001`: invocation audit records can remain indefinitely `running`
|
||||
|
||||
@@ -1304,9 +1515,9 @@ invariant to Stage 12.
|
||||
permissive umask for representative layout, manifest, artifact, log, and
|
||||
promoted-bundle paths; document platform/ACL limitations instead of assuming
|
||||
POSIX bits are universal.
|
||||
- Dependencies: Stage 6 owns configuration/default compatibility, Stage 7 owns
|
||||
adapter diagnostics, and Stage 13 should rank the risk against deployment
|
||||
assumptions.
|
||||
- Dependencies: Stage 6 confirmed there is no mode/config override or documented
|
||||
deployment privacy guarantee. Stage 7 owns adapter diagnostics, and Stage 13
|
||||
should rank the risk against deployment assumptions.
|
||||
|
||||
### `RSK-005`: remote publish locks are race-prone snapshots
|
||||
|
||||
@@ -1453,6 +1664,68 @@ invariant to Stage 12.
|
||||
filesystem symlink confinement. Stage 10 should reuse this result when
|
||||
reviewing manifest-first artifact trust.
|
||||
|
||||
### `RSK-009`: remotely discovered session configuration is left in system temporary storage
|
||||
|
||||
- Category: confirmed confidentiality/lifecycle risk.
|
||||
- Locations/invariant: `internal/app/config_loader.go` in `loadCommandConfig`
|
||||
and `internal/adapters/storage/temp_download.go` in
|
||||
`DownloadObjectToTemp`. Every successful temporary download must have an
|
||||
explicit owner and cleanup point, especially when it contains private session
|
||||
configuration.
|
||||
- Evidence: failed downloads remove their temporary file, but successful remote
|
||||
session fallback returns the path into `Config.SessionPath` and
|
||||
`SessionSource`. No caller removes it. Full execution later copies it to
|
||||
canonical `inputs/session.yml` yet leaves the original; plan, status,
|
||||
validate, and single-stage commands can leak one on every invocation. The
|
||||
ephemeral path can also be retained as source/spool provenance in a manifest.
|
||||
- Realistic scenario: a long-running worker repeatedly uses remote-only session
|
||||
configs. System temporary storage accumulates readable copies containing S3
|
||||
audio references and private campaign/session settings after commands finish,
|
||||
until an unrelated host cleanup policy happens to remove them.
|
||||
- Impact/likelihood/confidence: medium confidentiality and disk-lifecycle impact;
|
||||
deterministic for remote fallback; high confidence from all successful caller
|
||||
paths.
|
||||
- Estimated remediation scope and owner: medium app loader/command-lifetime
|
||||
change. Return an owned cleanup handle or bytes, defer cleanup at the command
|
||||
boundary after all consumers finish, and record canonical logical provenance
|
||||
rather than the ephemeral host path.
|
||||
- Test changes: assert removal after successful and failed full, read-only, and
|
||||
single-stage commands; preserve the existing failure cleanup assertion; verify
|
||||
manifests and reports do not retain a nonexistent system-temp source path.
|
||||
- Dependencies: do not remove the file before prepare or other consumers finish.
|
||||
`RSK-004` remains the broader runtime-mode disclosure boundary.
|
||||
|
||||
### `RSK-010`: filesystem secret loading follows links and reads non-regular entries
|
||||
|
||||
- Category: confirmed confidentiality/availability risk.
|
||||
- Locations/invariant: `internal/app/secrets_env.go` in
|
||||
`loadSecretsFromConfig`. A configured secrets directory should define a
|
||||
bounded set of regular secret files, not grant an ambient read capability
|
||||
through entry replacement or special file types.
|
||||
- Evidence: the loader skips only directory entries and invalid names, then
|
||||
calls `os.ReadFile` on the joined path. It follows a validly named symlink and
|
||||
accepts any other non-directory entry that `ReadFile` can open, without a
|
||||
regular-file/type/size check. The unreadable-entry test uses a broken symlink
|
||||
only to assert an error; it does not reject a working link before reading it.
|
||||
- Realistic scenario: a writable or incorrectly provisioned secrets directory
|
||||
contains `AUDITA_TOKEN` linked to an unrelated readable file, or a large/special
|
||||
entry. A more privileged invocation reads the target into process environment
|
||||
inherited by subprocess adapters, or incurs unbounded I/O/memory and blocks
|
||||
composition.
|
||||
- Impact/likelihood/confidence: high confidentiality/availability impact when
|
||||
directory ownership is weak; deployment-dependent likelihood; high confidence
|
||||
from the entry-type and read flow.
|
||||
- Estimated remediation scope and owner: small-to-medium app/filesystem boundary
|
||||
correction. Inspect without following links, admit regular files only, enforce
|
||||
a documented size bound, and use a handle-relative/no-follow read where the
|
||||
supported platforms allow entry replacement races to be closed.
|
||||
- Test changes: cover working and broken symlinks, FIFO/special entries where
|
||||
portable, oversized files, replacement races at the chosen primitive, valid
|
||||
newline trimming, and existing-environment precedence.
|
||||
- Dependencies: Stage 7 should verify downstream environment propagation but
|
||||
must not duplicate filesystem policy. `COR-003` owns mutation confinement,
|
||||
whereas this finding owns reads from the secret directory.
|
||||
|
||||
### `EFF-001`: restore repeatedly downloads the same objects during planning and execution
|
||||
|
||||
- Category: confirmed efficiency and clarity issue.
|
||||
@@ -1490,8 +1763,9 @@ invariant to Stage 12.
|
||||
|
||||
## Candidate Register
|
||||
|
||||
The remaining candidates require inspection by their named owners. Stages 2
|
||||
and 3 have promoted their confirmed defects and risks into the register above.
|
||||
The remaining candidates require inspection by their named owners. Completed
|
||||
behavior stages have promoted their confirmed defects and risks into the
|
||||
register above.
|
||||
|
||||
### `ARC-001`: `IODecl` is not a complete or consistently classified stage contract
|
||||
|
||||
@@ -1549,6 +1823,23 @@ and 3 have promoted their confirmed defects and risks into the register above.
|
||||
or derivation from loaded pointer identity. Any change must preserve the
|
||||
protocol correction selected for `COR-004`; do not add a post-pointer upload.
|
||||
|
||||
### `ARC-004`: notification configuration has no production transport consumer
|
||||
|
||||
- Category: architectural boundary/ownership candidate.
|
||||
- Evidence: pipeline config and the annotated example expose notification
|
||||
backend, recipient, and timeout settings, and validation parses the timeout.
|
||||
Production composition always supplies `notify.NoopSender`; the adapter package
|
||||
contains no configured production transport. The internal overview calls the
|
||||
stage a placeholder/default no-op, while public config surfaces do not make
|
||||
that limitation equally explicit.
|
||||
- Realistic risk: an operator configures a recipient and sees a successful notify
|
||||
stage without any delivery, or future code adds a transport while duplicating
|
||||
validation/composition policy in the stage.
|
||||
- Confirmation owner: Stage 7 should establish the notifier adapter contract and
|
||||
decide whether to construct a configured transport, reject non-placeholder
|
||||
values, or explicitly document/remove inert fields. Stage 8 owns ordinary
|
||||
notify-stage outcome semantics, not transport construction.
|
||||
|
||||
### `TST-001`: full race baseline fails in the concurrent transcribe test
|
||||
|
||||
- Category: test-suite execution candidate.
|
||||
@@ -1630,6 +1921,22 @@ and 3 have promoted their confirmed defects and risks into the register above.
|
||||
cache, manifest-last, and reporting in their current owners. Coordinate with
|
||||
`DUP-001`/`RSK-002` rather than adding another incomplete atomic writer.
|
||||
|
||||
### `DUP-004`: configuration repeats lexical relative-path policy
|
||||
|
||||
- Category: duplication/ownership candidate.
|
||||
- Evidence: `internal/config.validateRelativeSafePath` independently checks
|
||||
absolute paths, cleaned dot/traversal forms, and separators already represented
|
||||
by `internal/pathsafe` primitives, while adding config-specific field/error
|
||||
context. This is not currently a behavior defect and identity/root validation
|
||||
has deliberately different rules.
|
||||
- Realistic risk: new artifact fields can be accepted by one validator and
|
||||
rejected by another, multiplying the path-policy drift behind `COR-002` and
|
||||
future artifact configuration work.
|
||||
- Confirmation owner: Stage 11 should compare exact accepted-language and error
|
||||
contracts and decide whether config should delegate lexical mechanics to a
|
||||
narrow shared predicate. Do not collapse identifier, filesystem-root, or
|
||||
artifact relationship policy into one generic validator.
|
||||
|
||||
### `TST-003`: filesystem safety tests omit destination and lock-recovery boundaries
|
||||
|
||||
- Category: test-suite sufficiency candidate.
|
||||
@@ -1689,6 +1996,23 @@ and 3 have promoted their confirmed defects and risks into the register above.
|
||||
identical download boundary; retain the existing manifest-invalid case as the
|
||||
representative pre-rename validation failure.
|
||||
|
||||
### `TST-006`: injected runner environments can split configuration authority
|
||||
|
||||
- Category: test-seam fidelity candidate.
|
||||
- Evidence: `executeStages` receives both a resolved `cfg` argument and an
|
||||
optional injected `stage.Env`. It assigns `env.Config = cfg` only when the
|
||||
injected field is nil. With different non-nil values, layout, selected
|
||||
artifacts, and default stores derive from `cfg`, while secrets, adapter
|
||||
defaults, locks, and stage execution consume `env.Config`. Production passes
|
||||
no injected environment, and current tests generally use the same pointer by
|
||||
convention rather than an enforced invariant.
|
||||
- Realistic risk: a unit test can pass while exercising different stage enablement,
|
||||
credentials, paths, or timeouts from the manifest/layout under test, obscuring
|
||||
a production composition regression or creating an impossible fixture.
|
||||
- Confirmation owner: Stage 12 should inventory intentional divergent fixtures
|
||||
and either reject differing configs, always rebind to the resolved config, or
|
||||
replace the dual input with a constructor that makes ownership singular.
|
||||
|
||||
### `SIM-001`: runner terminalization and persistence ordering lack a narrow owner
|
||||
|
||||
- Category: simplification candidate.
|
||||
@@ -1760,6 +2084,18 @@ and 3 have promoted their confirmed defects and risks into the register above.
|
||||
| Previous-artifact readiness is equivalent to loading the prior current pair | confirmed as `COR-010` | Status/validate stop after pointer/manifest validation and do not resolve or check required artifact objects; their missing-previous-session policy also disagrees with optional planning behavior. |
|
||||
| Restore should roll back files written before a later failure | rejected as the current contract; retained as `RSK-006` | The documented operation is incremental and explicitly has no transaction or rollback. The risk is that the old manifest remains authoritative over partially replaced files and planning is not revalidated under the local lock. |
|
||||
| Ordinary manifest and previous-cache reads must always re-hash bytes | rejected as a universal rule at Stage 5 | Restore verifies remote/local equality when needed during classification, and prepare validates required previous artifacts before analysis. The confirmed defects concern generation binding, incomplete readiness checks, and lost source identity rather than a blanket checksum requirement. |
|
||||
| Known-field YAML decoding makes configuration strictly single-document | confirmed as `COR-012` | Known fields are enforced in the first document, but the second decode treats a successfully decoded trailing document as acceptable instead of requiring EOF. |
|
||||
| Any parseable duration is executable | confirmed as `COR-013` | Several adapters and stage parsers require positive timeouts, and the WhisperX constructor rejects a negative retry delay, while shared config validation checks syntax only. |
|
||||
| Populated S3 fields are a compatible implicit backend selector | confirmed as `COR-014` | The documented backend field is operator authority. Silently selecting S3 after an unknown spelling hides invalid configuration and makes validation disagree with construction. |
|
||||
| `--previous-session-id` is only a conditional consistency hint | confirmed as `COR-015` | The option is described and modeled as an expected identifier. Ignoring it when the session omits the field defeats the only CLI-provided expectation. |
|
||||
| Successful temporary remote-session download has caller-owned cleanup | confirmed as `RSK-009` | The helper transfers ownership on success, but no command caller removes the file after config consumers finish and ephemeral provenance can be persisted. |
|
||||
| A trusted secrets directory makes link/type checks unnecessary | confirmed as `RSK-010` | Deployment ownership is not validated by the process and mistakes are realistic. The loader follows working links and admits non-regular entries without a bound. |
|
||||
| All adapter constructors are unconditional expensive/external work | rejected at Stage 6 | Default HTTP/subprocess wrappers do no connection or process work at construction. Notarius, object storage, and remote locks are conditional on selected behavior that needs them. |
|
||||
| Repeated adapter constructors and single-stage commands require immediate consolidation | rejected as standalone Stage 6 findings | Constructors translate distinct protocols, and single-stage dispatch already funnels through `runSingleStageCommand`. Similar shape does not establish duplicated policy. |
|
||||
| Repeated filesystem secret scans are an efficiency defect | rejected as material at Stage 6 | Object-store construction can repeat a deterministic bounded directory scan, but the scan is small, preserves secret-before-adapter ordering, and no material latency or external cost was established. |
|
||||
| Notification settings are consumed because notify succeeds | retained as `ARC-004` | Production success is supplied by a no-op sender regardless of backend/recipient. Stage 7 must decide whether this is an intentional placeholder contract or an inert operator surface. |
|
||||
| Injectable runner `Env` always represents the resolved config | retained as `TST-006` | Production composition does, but a non-nil injected `Env.Config` is retained and can differ from the explicit config used by other runner setup. |
|
||||
| Configuration relative-path validation is distinct from shared lexical safety | retained as `DUP-004` | Config adds field context, but its cleaned relative-path language overlaps `pathsafe`; Stage 11 must compare semantics before centralizing mechanics. |
|
||||
|
||||
## Unresolved Questions And Follow-Up
|
||||
|
||||
@@ -1800,6 +2136,18 @@ and 3 have promoted their confirmed defects and risks into the register above.
|
||||
identities?
|
||||
- How should publish metadata retain a stable source-to-destination mapping for
|
||||
previous artifacts without reintroducing mutable configuration authority?
|
||||
- Should `--previous-session-id` remain a strict expectation or become an
|
||||
explicit override, and which compatibility cases already depend on omission?
|
||||
- Is any storage backend other than explicit S3 or disabled/local behavior part
|
||||
of the supported configuration language?
|
||||
- What ownership and maximum-size guarantees do deployment packages establish
|
||||
for the filesystem secrets directory, and can all supported platforms enforce
|
||||
no-follow regular-file reads?
|
||||
- Should remote session loading retain bytes until canonical preparation or
|
||||
expose an owned temporary handle whose cleanup spans every command consumer?
|
||||
- Is notification configuration intentionally reserved while delivery stays a
|
||||
no-op, or should non-placeholder values fail validation until Stage 7 defines
|
||||
a production transport?
|
||||
- Which native CI runner limitations explain the absence of validation jobs in
|
||||
the tag-only release workflow? Stage 12 owns the automation conclusion.
|
||||
|
||||
@@ -1986,3 +2334,41 @@ No accepted risks or final audit conclusions are recorded yet.
|
||||
prefix objects; scenario 6 confirms partial incremental replacement, old-
|
||||
manifest authority before the final install, and restored state despite a
|
||||
later report failure. Rollback and automatic retry are intentionally absent.
|
||||
|
||||
### Stage 6
|
||||
|
||||
- Contracts and code reviewed: development guide, all repository policy,
|
||||
configuration/CLI/example and internal adapter contracts; process entry and
|
||||
dispatch; pipeline/campaign/session discovery; strict YAML loading, defaults,
|
||||
resolution, templates, and every validation family; secrets loading; runner,
|
||||
adapter, object-store, lock, and remote-session composition.
|
||||
- Graph/source evidence: exact snippets and caller/data-flow traces for loaders,
|
||||
defaults, `Resolve`, `Validate`, duration and storage checks, command selection,
|
||||
remote fallback, secret loading, `executeStages`, conditional constructors,
|
||||
temporary downloads, and injected environments. Direct source/text inspection
|
||||
covered YAML tags, help text, documentation, maintained examples, and lifecycle
|
||||
details the graph could not distinguish.
|
||||
- Validation: `go test -count=1 ./internal/config ./internal/app ./cmd/narratio`
|
||||
passed (`internal/config` 0.099 s, `internal/app` 0.818 s, CLI has no test
|
||||
files; 2.11 s command wall time). `go vet ./...` passed (1.88 s command wall
|
||||
time).
|
||||
- Conclusions: explicit/default selection precedence, empty-value semantics,
|
||||
relative-path anchoring, resolution and validation order, every operator field
|
||||
family and runtime consumer, secret propagation, enabled/disabled adapter
|
||||
composition, resource ownership, and maintained example validity are recorded
|
||||
above. No live credentials or external services were required.
|
||||
- Findings: confirmed `COR-012`, `COR-013`, `COR-014`, `COR-015`, `RSK-009`,
|
||||
and `RSK-010`; added `ARC-004`, `DUP-004`, and `TST-006` with named later
|
||||
owners.
|
||||
- Explicit no-finding conclusions: pointer-valued defaults preserve explicit
|
||||
false/zero; explicit empty modules remain distinct from omission; Notarius
|
||||
paths are anchored to the pipeline; external work is conditionally composed;
|
||||
default client/runner construction opens no closeable resource; secret values
|
||||
are not persisted or logged; maintained examples use non-secret placeholders
|
||||
and are executable under representative sessions. Repeated bounded secret
|
||||
scans and protocol-specific constructors do not establish material efficiency
|
||||
or duplication defects.
|
||||
- Documentation/example disposition: config defaults and maintained example
|
||||
structure match implementation. Notification backend/recipient settings are
|
||||
the material drift because production always uses a no-op sender; `ARC-004`
|
||||
assigns the transport/documentation decision to Stage 7.
|
||||
|
||||
Reference in New Issue
Block a user