Record the first six remediation assessments

This commit is contained in:
2026-11-05 12:38:10 +00:00
parent 21fea46fb3
commit a85a6788b4

View File

@@ -12,7 +12,9 @@ The [catalog's six accepted decisions](audit/catalog.md#resolved-decisions)
remain authoritative. This assessment selects ways to implement those decisions;
it does not reopen them or repeat the repository-wide audits. Catalog numbers
below identify the relevant findings, not additional implementation stages.
All assessment topics are initially pending.
Assessment started on 2026-11-05 at clean commit
`21fea46fb3b0ce9483db2399c9587f4a2323d687`. Assessment records below distinguish
completed analysis from still-pending implementation.
## Assessment Method And Outputs
@@ -75,6 +77,32 @@ work to schedule, not permission to install one during assessment or to stop
independent source analysis. Retain the accepted separation between maintainer
selection and consumer minimum; do not add consumer advisories or scan gates.
### Assessment Record
**Status:** Assessed; no environment installation is needed.
**Evidence:** Default `/snap/bin/go` reports Go 1.27.0, but the existing module
cache contains executable Go 1.27.3 at
`/home/eric/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.27.3.linux-amd64/bin/go`;
invoking it reports `go1.27.3 linux/amd64`. Other inspected SDK/cache installs
are older. `go.mod` declares 1.27.0, ConfigKit 1.1.0, jsonschema/v6 6.0.2, and
ReleaseKit 0.9.0. `scripts/releasekit.sh` sets `GOTOOLCHAIN=local`, so its
toolchain is selected by `PATH`, not automatically upgraded.
**Recommendation and alternatives:** Put the existing 1.27.3 binary directory
first on `PATH`, with `GOTOOLCHAIN=local` and `GOWORK=off`, for subsequent
assessment commands and implementation validation. The authoritative affected
ranges for [GO-2026-6611](https://pkg.go.dev/vuln/GO-2026-6611) and
[GO-2026-6809](https://pkg.go.dev/vuln/GO-2026-6809) exclude 1.27.3: the two
specific audited exposures are addressed by that selection. This is a focused
version/applicability reassessment, not a new full vulnerability scan or a claim
about consumer binaries. Continuing with default 1.27.0 retains the identified
exposure and is not recommended; installation is unnecessary.
**Plan handoff:** Clarify ordinary development toolchain selection in
`docs/development.md`; leave `go 1.27.0` unchanged. Record the actually selected
patch when validating remediation. No consumer advisory or recurring scan gate.
## 2. Specify Direct ConfigKit Source Handoff
**Catalog:** 3, 5; informs 24.
@@ -92,6 +120,38 @@ a replacement converter, pre-validation pass, or upstream feature. Existing
upstream ownership evidence is sufficient; exhaustive ConfigKit limit testing
is not an assessment deliverable.
### Assessment Record
**Status:** Assessed; remove the compatibility adapter without replacement.
**Evidence:** `ProfileSource` (`profiles.go:29`) and `BackendSource`
(`backends.go:82`) already project optional typed fields into built-in values
and wrap definitions in `[]any`. Their only recursive compatibility dependency
is `providerMapForMapping(config.ExtraParams)`. The pinned ConfigKit 1.1.0
`source.go:Mapping` promises immediate capture and deferred unsupported/cyclic
diagnostics. Its `internal/engine/source.go:Mapping` builds an owned document
and retains `mapIssue`; `readSource` reports it when consumed. PromptKit's
`provider_values.go` instead expands maps/slices before that bounded handoff
and parses `json.Number` through a lossy float fallback. Existing
`sources_contract_test.go` mainly protects flat-map ownership, not this broader
input boundary.
**Recommendation and alternatives:** Assign `config.ExtraParams` directly into
the ordinary fields map in both helpers, then call `configkit.Mapping` as now.
Delete `provider_values.go` after its two callers are replaced. A shallow map
copy adds no capture protection beyond Mapping; another recursive converter or
raw validation would retain the responsibility being removed. Runtime overrides
continue using their separate JSON-value boundary.
**Plan handoff:** Cover both helpers with nested built-in maps/`[]any`, mutation
after source creation, and deferred failures for representative `json.Number`,
named scalar/typed container, pointer, and cyclic inputs. Keep construction
order/optional-field coverage. Explain caller conversion to supported forms:
for example, `[]string` becomes `[]any` of strings; callers choose and check
numeric conversion themselves. Do not assert preserved numeric spelling or
duplicate ConfigKit's entire resource-limit suite. Source-helper GoDoc and
migration notes own the changed accepted-value boundary.
## 3. Specify Self-Contained Schema Resolution And Compilation
**Catalog:** 6, 15, 21; coordinates with 32.
@@ -116,6 +176,55 @@ direct-validation cases. Specify how the provider receives the self-contained
document unchanged in meaning. No bundler, schema rewriting prototype, or
live provider compatibility experiment is needed.
### Assessment Record
**Status:** Assessed; use explicit root registration and a no-external-resource
loader, with coverage for unused schema definitions.
**Evidence:** `internal/validate/standard_validator.go` has four compilation
paths; its root loaders already preserve JSON numbers, validate the 2020-12
dialect, and check source containment/context. `Runner.prepareValidation` uses
only `ValidationPreparer`, despite storing `Validator`. In pinned jsonschema
6.0.2, `Compiler.AddResource` registers the decoded document, `roots.go` collects
embedded IDs/anchors, and `loader.go` consults registered resources and bundled
metaschemas before the custom loader.
An offline dependency-contract probe in
`/tmp/promptkit-assessment-aqn3B8/schema.go`, run with Go 1.27.3 using
`go run -mod=readonly` and `GOPROXY=off`, registered one document and installed a
loader that only counts/rejects external loads. Fragment, anchor, recursion,
absolute self-ID, embedded child-ID, and standard metaschema cases compiled
with zero external loads; a real external reference failed with one load.
Crucially, an unused `$defs` entry with an external reference did not trigger a
load until compiled explicitly. An example payload containing a literal `$ref`
did not trigger loading. A denying loader alone therefore does not establish
that every declared schema is self-contained.
**Recommendation and alternatives:** Retain source-specific root loading, then
share `compileSchemaDocument(ctx, resourceURL, document)` returning the compiled
executor. Register the root, retain Draft2020, and install a context-aware loader
returning an actionable external-resource error without filesystem/network I/O.
Keep compiler resolution authoritative for IDs and anchors. To cover otherwise
unused declarations, enumerate schema locations using a small, explicit table
of the pinned 2020-12 schema-bearing keyword positions (including its supported
legacy positions) and compile those locations with the same compiler/cache.
Do not traverse annotation/example objects as schemas. This bounded policy walk
is not a URI resolver, bundler, or general schema framework. A lexical ban on
non-fragment references rejects embedded IDs; root-only compilation misses the
demonstrated unused-definition case. Neither is recommended.
**Plan handoff:** Specify escaped JSON-pointer location enumeration and retained
embedded-ID/dynamic-reference semantics in focused cases; preserve dialect and
number handling. Both direct schema callbacks call the shared compilation only
after instance decoding, while prepared paths call it eagerly as before. Remove
external loaders, not root path containment/read helpers. Change runner fields
and constructor parameters to `validate.ValidationPreparer`, retaining the nil
no-op and nil-plan error behavior. Validator constructors can return their
concrete pointer types so existing direct `Validate` callers and preparation
both compile; no public facade signature changes are needed. Replace external
resource success fixtures with early-error cases, retain independent root-source
tests, and add public `WithSchemaFile` option/replacement/error coverage.
## 4. Specify Ownership And Retention Remedies
**Catalog:** 4, 16, 18.
@@ -133,6 +242,37 @@ allocations comparing unequal. Identify the session-string detachment point.
List relevant concurrent and mutation regressions for implementation, without
building a new ownership framework or changing lifetimes during assessment.
### Assessment Record
**Status:** Assessed; narrow references rather than adding lifecycle machinery.
**Evidence:** `artifact_reader.go:publicArtifactReaderAdapter.Read` copies the
returned body only after the consumer call returns. `PreparedExecution` in
`internal/usecase/prepared_execution.go` retains `owner *Runner` even after its
mutex-protected claim/discard drops the payload; `Details` uses separately
captured data. `Runner` owns repositories, clients, and admission. Namespace
views delegate to the same internal engine/runner. `NormalizeSessionID` in
`internal/domain/session.go` returns `strings.TrimSpace(raw)` directly.
**Recommendation and alternatives:** Document successful artifact return as
ownership transfer, as accepted, and keep adapter copying. Replace the prepared
handle's runner pointer with a pointer to a non-zero-sized private identity
object allocated once by each runner constructor and copied into its handles.
Keep that immutable token after claim/discard; it retains no runner graph and
preserves foreign-owner validation order without a new owner-field race.
Clearing a mutable runner pointer is viable but complicates synchronized checks
and error ordering unnecessarily. Clone the validated nonempty normalized
session string with `strings.Clone` before returning it; this detaches padded
input without imposing a new limit.
**Plan handoff:** Retain ready-to-claimed/discarded transitions under the current
mutex, private payload clearing, one-shot execution, and detached details.
Protect cross-engine rejection, same-engine namespace views, concurrent
claim/discard, and details after discard/use. Artifact regressions must use
independently returned buffers and avoid claiming safety for contract-violating
readers. Existing mutation, trimming, Unicode, hash, and wire assertions remain
the semantic oracle; a GC timing assertion is unnecessary.
## 5. Specify Error Ownership And Preparation-Stage Information
**Catalog:** 7, 8, 9; informs 25.
@@ -150,6 +290,57 @@ boundaries where possible. Give a small expected-outcome matrix of public
category, retained cause, diagnostic completion, and dispatch behavior. Retained
causes may match multiple sentinels; category exclusivity is not the target.
### Assessment Record
**Status:** Assessed; carry ownership and preparation provenance explicitly.
**Evidence:** `errors.go:publicErrorFor` uses whole-chain `errors.Is` in a fixed
order; `mapPublicError` also searches nested capacity/exhaustion/provider types
before normal mapping. `wrapGenerationError` and artifact loading add operation
sentinels but no separately readable owning category. In
`internal/llm/openai_compatible_client.go:decodeOpenAIChatResponse`, both decoder
calls replace non-context errors with malformed-response messages. In
`generation.Coordinator.generate`, `PrepareGeneration` fails before any capacity
attempt, while `SmokeTester.runInference` concludes every generation error as
report data. `GenerationEvidence` currently lacks preparation-stage information.
**Recommendation and alternatives:** Add a small internal use-case operation
error carrying category and cause, with `Unwrap`/`Is` behavior. Apply it at owning
boundaries and let facade mapping select that category independently of nested
causes. Preserve legitimate direct not-found mappings when assigning the
category. Keep specialized public capacity/provider/exhaustion projections, but
do not let their presence inside a collaborator cause erase the outer owning
category. A globally reordered sentinel switch cannot express this distinction.
Add `PreparationFailed bool` to internal `GenerationEvidence`, set only at the
built-in `PrepareGeneration` failure return. Diagnostic orchestration checks it
after caller cancellation and returns `ErrInvalidRequest` with the cause.
Do not infer this stage from error identity or attempt count; injected failures
and intentional missing-key/capacity reports keep their established semantics.
This needs no new public field or retry policy.
Wrap the successful-response reader with a small read-error recorder beneath
the existing limited reader. At decoding/EOF verification failure, prefer an
observed context error; preserve size overflow as malformed; otherwise propagate
an actual non-EOF reader failure through `requestFailedError`. Parser-generated
syntax/type/truncation failures, EOF-truncated JSON, and actual extra values
remain malformed. Distinguish errors returned by the reader from those created
by `encoding/json`, rather than categorizing every `io.ErrUnexpectedEOF` alike.
Retain redacted default formatting and exact underlying read causes.
**Plan handoff:** Protect these expected outcomes at the public boundary:
| Case | Owning category / result | Retained evidence |
| --- | --- | --- |
| Client returns child missing-prompt error | `ErrLLMGenerate`; error | Child cause remains matchable, including its category. |
| Reader returns the same child error | `ErrArtifactLoad`; error | Child cause remains matchable. |
| Direct missing prompt/profile | Existing not-found category; error | No conversion into a generic load failure. |
| 2xx body read fails during decode or final EOF check | Generation plus transport; diagnostic transport failure | Original reader cause, no raw body/detail formatting. |
| Invalid/truncated JSON, extra value, size overflow | Existing malformed-response behavior | No new retry or parser-detail disclosure. |
| Built-in diagnostic preparation fails | `ErrInvalidRequest`; not a completed diagnostic | Zero dispatch and original preparation cause. |
| Missing required credential/capacity report | Existing completed diagnostic outcome | No inferred construction failure. |
| Caller cancellation | Existing cancellation/error handling | Cancellation takes precedence over publishing success. |
## 6. Specify Authentication Overflow Reporting And Adapter Ownership
**Catalog:** 2, 26, 27; uses topic 5's error/stage conclusions.
@@ -172,6 +363,52 @@ Recommend the outbound protocol-adapter seam and narrow target-copy operation.
Keep the body-bound fix independently implementable before adapter relocation;
do not add a public budget option or general diagnostic plugin framework.
### Assessment Record
**Status:** Assessed; one reason constant suffices, without a new report field.
**Evidence:** Public/domain smoke reports already have `Reason`, `Failure`,
nullable `CredentialsAccepted`, HTTP status on attempts, and timing. Current
`smoke_authentication.go` reads a body only to recognize HTTP 200; 401 establishes
rejection without reading it. Other statuses retain their existing outcome.
`smoke_tester.go` copies these facts into attempts and `finishHTTP` handles
caller cancellation. The outbound client already owns authenticated GET and
HTTP evidence. The broad domain request/report cloners are used only by their
self-tests and a target-only copy respectively, as recorded in audit 11.
**Recommendation and alternatives:** Add matching public/domain
`SmokeTestReasonAuthenticationResponseLimitExceeded` constants with value
`authentication_response_limit_exceeded`. On a 200 recognition overflow set that
reason, existing `ExecutionFailureOther`, acceptance nil, and retain HTTP/timing
facts; conclude with a nil Go error unless caller cancellation applies. A new
general failure enum or report Boolean is unnecessary: the existing reason
already provides an explicit machine-readable diagnostic. Keep 401 rejection
and non-200 behavior independent of unused response bodies.
Read at most allowance plus one byte with `io.LimitReader`/`io.ReadAll`, detect
length greater than 1 MiB before JSON recognition, and close without draining.
Do not trust `ContentLength`. Existing bounded bytes plus one-byte lookahead are
sufficient; streaming the small account parser adds complexity without need.
Cancellation takes its existing precedence; overflow is not malformed content.
Move account-response recognition and bounded response ownership into an
`internal/llm` OpenRouter authentication operation, returning HTTP evidence,
optional acceptance, overflow/unrecognized indicators, and operational error.
Move default endpoint derivation into a small llm adapter helper used by
selection; explicit overrides remain unchanged. Use-case orchestration maps
those factual results to report reasons/completion and consumes a narrow
authentication capability alongside reachability HTTP capability. No reverse
dependency or plugin registry is needed. Replace target-only full-report cloning
with direct target/provider-map detachment; retain facade projection and active
backend cloners.
**Plan handoff:** Fix and test the byte bound before relocation. Include exact
limit versus limit-plus-one, escaping/invalid bodies, inaccurate declared length,
closure, 401 without body consumption, context interruption, redaction, and
detached report values. Update reason GoDoc/public conversion and focused
diagnostic documentation in the same implementation unit. No secret-bearing
body, header, or provider detail is added to report data.
## 7. Assess JSON Copying Compatibility And Provider Overlay Cost
**Catalog:** 19, 24; uses topic 2's converter-removal boundary.