From 62bf82a56bde5a2fa84e1ae7a42f212cf4578f11 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 5 Nov 2026 04:37:26 +0000 Subject: [PATCH] Add audit remediation roadmap --- docs/roadmap/audit-remediation.md | 366 ++++++++++++++++++++++++++++++ 1 file changed, 366 insertions(+) create mode 100644 docs/roadmap/audit-remediation.md diff --git a/docs/roadmap/audit-remediation.md b/docs/roadmap/audit-remediation.md new file mode 100644 index 0000000..999ddff --- /dev/null +++ b/docs/roadmap/audit-remediation.md @@ -0,0 +1,366 @@ +# Audit Remediation Roadmap + +## Purpose And Status + +Prepare PromptKit's audited implementation for a focused round of correctness, +configuration simplification, resource-use improvements, and internal cleanup. +This document defines the target and work boundaries for a subsequent staged +implementation plan; it is not that plan or a description of completed changes. + +Baseline: repository commit `bd3dfa9`, with the +[audit catalog](audit/catalog.md) covering 41 source findings consolidated into +34 remediation entries. All six catalog decisions have been accepted; their +implementation remains pending. The catalog and supporting audits own finding +evidence and decision rationale. This roadmap owns remediation organization, +dependencies, completion evidence, and subsequent workstream status. Earlier +remediation suggestions in the source audits do not override accepted decisions. + +All workstreams below are pending. Catalog numbers are traceability identifiers, +not a required implementation order or 34 separate delivery stages. The scope +contains confirmed defects, documentation/test gaps, and maintainability work; +those categories do not have equal urgency or automatically become release gates. + +## Targeted End State + +PromptKit remains an application-neutral Go library with the existing public +facade and ConfigKit composition boundary. The intended changes are: + +- Programmatic backend/profile sources hand supported values to ConfigKit + directly, removing PromptKit's recursive compatibility conversion. +- Schemas supplied for validation and native structured output are self-contained + documents, while in-document references remain supported. +- Artifact-reader ownership, public errors, and smoke-test outcomes accurately + express the owning operation and collected evidence. +- Authentication response consumption is bounded by the selected internal + allowance, without treating overflow as rejected credentials. +- Identified unnecessary retention, repeated traversal, and temporary allocation + are reduced with proportionate changes that preserve established behavior. +- Production execution and its tests use the real generation coordinator; + obsolete adapters, state, helpers, and duplicate test expectations are removed. +- Current documentation describes actual behavior, and deliberate compatibility + changes are explained where consumers encounter them. + +The [resolved decisions](audit/catalog.md#resolved-decisions) are authoritative +for the selected changes. They do not authorize stronger numerical precision, +provider compatibility, scheduling, hash stability, or performance guarantees. + +## Scope Boundaries + +Keep the work within the [architecture policy](../policy/architecture.md) and +[configuration format boundary](../formats.md). ConfigKit owns source composition +and its raw-source diagnostics; PromptKit validates its final effective values. + +This round does not include a schema bundler, a new local-only schema mode, an +upstream ConfigKit extension, a numeric conversion subsystem, new inference +limits or application deadlines, or a general reflection/validation framework. +[Schema bundling](future.md#schema-bundling) remains a future idea. Runtime +provider overrides retain their distinct supported-value and copying behavior. + +Refactoring does not authorize expanding recognized template expressions, +introducing additional retries, adding pagination or runtime inventory caches, +or redesigning the capacity manager. Tests should protect established semantics, +not turn incidental implementation choices into new requirements. Existing +privacy and ownership boundaries remain applicable throughout. + +## Workstreams + +### A. Maintainer Toolchain Baseline + +**Catalog:** 1. **Decision:** 5. + +**Current gap:** The audited local toolchain predates the networking fixes +identified by the security audit. Toolchain selection and consumer compatibility +must remain separate concerns. + +**Work:** Use the latest locally available Go installation for ordinary builds, +tests, and release validation. Update the local installation as needed for the +identified fixes and repeat the focused assessment with the selected version. +Make the ordinary-development instruction explicit in contributor guidance. +Follow the [release procedure](../release.md#prepare-the-candidate) and +[consumer-minimum policy](../policy/release.md#consumer-minimum-and-release-toolchain): +normally declare the initial stable release of the selected series, advance the +series in minor releases, and retain the existing documented exceptions. Merely +selecting a newer patch for validation does not raise the consumer minimum. + +**Completion evidence:** Record the toolchain actually used and the focused +reassessment outcome, distinguishing resolved findings from any remaining +applicability. This does not establish a new recurring scan gate. Advising +consumers about generally applicable Go vulnerabilities is outside PromptKit's +responsibility; no such advisory work belongs in this roadmap. + +### B. Configuration And Schema Simplification + +**Catalog:** 3, 5, 6, 15, 21. **Decisions:** 2 and 4. + +**Current gap:** Convenience-source conversion broadens ConfigKit's accepted +input forms by recursively rebuilding them, causing silent numeric changes and +potentially exponential expansion. Schema validation loads external resources +locally but sends only the root document to the provider. Compilation is +repeated across direct/prepared and host-file/`fs.FS` paths. + +**Work:** + +1. Remove the provider-value compatibility adapter from `ProfileSource` and + `BackendSource`, retaining ordinary field projection. Pass provider maps + directly to ConfigKit `Mapping` for snapshotting and deferred diagnostics. + Callers supply supported built-in scalars, nil, `map[string]any`, and `[]any` + recursively. Do not replace the removed adapter with a precision checker, + pre-validation pass, or memoized converter. +2. Require self-contained schema documents. Continue loading the chosen root + schema from its configured source; reject references requiring another + document with an actionable preparation error. Preserve supported references + within the document, including recursion where otherwise supported. Apply + the selected resource policy consistently across direct and prepared schema + compilation without changing malformed-output short-circuiting into eager I/O. +3. Simplify and share the remaining compilation lifecycle behind the needed + source adapters. Express the runner's preparation dependency in its internal + interface rather than discovering it through a runtime assertion. Preserve + source containment, cancellation, frozen plans, and existing validation + semantics outside the selected external-resource removal. +4. Cover `WithSchemaFile` through public construction and preparation/validation, + including option precedence and relevant file errors. Coordinate removal of + its unused private helper result with workstream F. + +**Completion evidence:** Focused source-helper cases demonstrate supported-value +handoff, immediate source capture, and ConfigKit's deferred unsupported/cyclic +value diagnostics. Include formerly converted input forms to demonstrate the +migration boundary; do not duplicate ConfigKit's entire limit suite. Schema +cases establish external-reference rejection before provider dispatch, retained +in-document references, and an outbound self-contained constraint usable without +other local files. Retain independent source-path and direct/prepared coverage. +These checks establish packaging completeness, not universal provider support. + +**Compatibility:** Explain caller conversion of `json.Number`, named scalars, +typed containers, arrays, and nested pointers into ConfigKit-supported forms. +Explain removal of external schema loading. No arbitrary-precision promise or +change to runtime request overrides accompanies this simplification. + +### C. Ownership, Errors, And Diagnostic Evidence + +**Catalog:** 2, 4, 7, 8, 9, 11, 26, 27. **Decisions:** 1 and 3. + +**Current gap:** The documented artifact buffer lifetime permits races; nested +errors can obscure their owning operation; response read failures can be +misclassified; some pre-dispatch smoke failures produce a completed report; +and successful authentication responses are read without a byte bound. + +**Work:** + +1. Align artifact GoDoc, adapters, and tests with successful `Read` transferring + the artifact and mutable body storage to PromptKit. Readers stop mutating + that storage, including retained aliases. Keep the interface signature and + existing defensive copy initially. Readers may reuse private buffers when + they return an independent copy; no release protocol is introduced. +2. Preserve the owning public error category independently of nested causes, + retaining appropriate `errors.Is`/`errors.As` access. Do not require category + exclusivity: retained causes can make more than one category match. Preserve + meaningful direct-not-found versus load-failure distinctions. +3. Distinguish successful-response body I/O failures from malformed JSON at both + initial decoding and end verification. Preserve the redacted transport + boundary, cancellation precedence, and genuine malformed, truncated, or + oversized-content behavior without adding retries. +4. Return local inference-diagnostic construction failures as `ErrInvalidRequest` + using explicit preparation-stage information. Preserve completed diagnostics + for missing credentials, capacity outcomes, and unsupported/inapplicable + operations; absence of dispatch alone does not imply an invalid request. +5. Define the authentication response allowance as a named internal constant of + 1 MiB (1,048,576 bytes). Enforce against actual reads regardless of declared + length, detect overflow, and close without unbounded draining. Report an + explicit limit diagnostic with credential acceptance unknown and retain + available HTTP status/timing. Choose its concrete report representation + during implementation design; overflow is not malformed JSON or credential + rejection. No public configuration knob is added now. +6. Move OpenRouter-specific authentication request/response interpretation into + the outbound adapter. Leave selection, applicability, completion, and neutral + reporting in the use case. Preserve anonymous probing and credential/body + privacy. Narrow smoke copying to actual consumers, removing unused request + cloning and replacing full-report cloning used only to detach a target. +7. Exercise `RunRequest`'s privacy formatting and JSON omission boundary with + synthetic markers and the relevant value/pointer forms. + +**Completion evidence:** Focused concurrent artifact tests follow the selected +ownership contract; they do not promise safety for readers that violate it. +Offline regressions cover composed public causes, both response read-failure +locations, construction failures with no dispatch, and authentication boundary +and overflow cases including missing/inaccurate lengths. Assert diagnostic facts +and privacy rather than complete incidental serialized shapes. Preserve active +backend copying, facade projection, and provider-map error behavior. + +**Dependencies:** The authentication bound can land before protocol relocation; +cleanup must not delay the defect remedy. Correct error/stage behavior before +restructuring generation accounting in E. + +### D. Resource Use And Inspection Efficiency + +**Catalog:** 10, 16, 17, 18, 19, 20. + +**Current gap:** The audited paths repeatedly copy requirement evidence, retain +unneeded backing storage or engine ownership, serialize unbounded diagnostic +text before truncation, reclone overlay descendants, and scan complete catalogs +for namespace-filtered listings. + +**Work and completion evidence:** + +| Area | Intended remedy | Evidence and limits | +| --- | --- | --- | +| Requirement evidence (10) | Accumulate or merge evidence without repeatedly copying growing sets. | Exercise representative left-heavy conditions; preserve classification and conservative coverage without new expression limits. | +| Prepared handles (16) | Prefer a small standalone ownership identity that does not retain the runner graph. | Preserve foreign-owner checks, one-time claim, namespace-view behavior, details, and concurrent claim/discard behavior. Identity cannot rely on distinct zero-sized allocations comparing unequal. | +| Repair feedback (17) | Encode bounded feedback incrementally rather than fully serializing then truncating. | Preserve output semantics, stored diagnostics, and previous candidate; measure large and heavily escaped diagnostics. No exact peak-heap guarantee or new input limit. | +| Session IDs (18) | Detach the accepted normalized string from a larger caller allocation. | Preserve trimming, content/code-point validation, wire values, and hashes; no raw-input restriction. | +| Provider overlays (19) | Avoid repeated descendant cloning while preserving owned results. | Compare representative nested overlays and preserve recursive-map, null, array, and isolation behavior. | +| Namespace listings (20) | Capture buckets or ranges to query the relevant inventory subset. | Evaluate sparse/absent matches, ordering, exact identity, cancellation, caller-owned results, and construction/retained-memory cost. | + +Measurements guide proportionate remedies, particularly for 17, 19, and 20; +they are not new performance contracts, benchmark thresholds, or recurring +release gates. If a proposed optimization's benefit does not justify its +complexity, record the evidence and an explicit disposition rather than silently +marking it implemented. Do not broaden the remediation merely to improve a +synthetic benchmark. Coordinate evidence accumulation with E's traversal work +and overlays with E's compatible-copying work. + +### E. Internal Coordination And Test Assembly + +**Catalog:** 22, 23, 24, 25, 34. **Decision:** 6. + +**Current gap:** Large functions combine distinct responsibilities, JSON copying +is duplicated across boundaries with different policies, and a test-only legacy +capacity wrapper remains a production constructor. + +**Work:** + +- Extract focused configuration validation/materialization helpers with visible + inputs and ownership. Preserve final-handoff validation, presence/null rules, + origins, and meaningful error precedence without pinning incidental ordering + of independent failures. +- Separate requirement recognition/evaluation from reachability and evidence + bookkeeping, sharing `if`/`with` orchestration where appropriate. Preserve + scope differences, inactive computed-name behavior, and independent comparison + with the actual renderer. This does not expand inspection into an interpreter. +- Consolidate structural copying only where policies genuinely agree. Keep + facade projection and runtime validation distinct; preserve existing types, + nils, supported numbers, schema keys, and ownership. Infallible projection + must not acquire new errors or silent fallbacks. Retain small separate walkers + if sharing would require a configurable reflection framework. B's removed + source compatibility adapter must not migrate into this shared machinery. +- Separate generation attempt results and accounting from dispatch/retry control + flow with small internal helpers. Preserve built-in/injected differences, + permit release before callbacks or backoff, frozen requests, existing retry + classification, private evidence, and known/unknown usage distinctions. +- Move the initial/repair runner scheduling test to the production generation + coordinator, then remove the legacy capacity client wrapper. Preserve useful + FIFO, cancellation, independent-pool, and panic-release scenarios with their + owning components. Keep any useful one-shot scaffolding test-only; do not wrap + the coordinator in another permit-acquisition layer. + +**Completion evidence:** Existing meaningful behavioral tests and focused cases +for changed seams continue to exercise real internal collaborators. Tests no +longer require the retired production wrapper. Review demonstrates simpler +ownership/control flow; lowering complexity metrics alone is not an acceptance +criterion. No capacity-manager redesign or public DTO unification is needed. + +### F. Documentation, Representation, And Test Maintenance + +**Catalog:** 12, 13, 14, 28, 29, 30, 31, 32, 33. + +**Current gap:** Several comments contradict implemented behavior, expectations +are duplicated or over-specified, and internal fields/helpers have no consumer. + +**Work:** + +- Correct credential-inspection GoDoc and its example: explicit profile credential + environment selection can coexist with required credentials; only the + backend-derived default is suppressed in that case. Correct provider-map wire + documentation to distinguish configured recursive overlays from runtime + replacement. Remove stale trimmed-ID and prompt-version comments. +- Give complete bundled-catalog expectations one independently specified test + owner. Retain smaller tests for distinct loading, wire, and smoke behavior, + using controlled fixtures where bundled content is irrelevant. +- Replace literal artifact-hash algorithm assertions with documented opaque + equality/ownership and byte/size behavior; keep benchmark setup outside timed + work. This does not require changing the artifact hash algorithm. +- Remove unused backend presence records and `QueueCapacitySet`, preserving + effective queue defaults, explicit zero, provenance, and needed profile/input + presence tracking. +- Remove duplicated captured prompt output-format and unused message-file fields. + Preserve actual `content_file` loading, origins, and override-default behavior. + Account for observable definition-hash changes without promising permanent + encoding or bit-identical hashes across this change. +- Remove unused private reachability endpoint state and the unused schema-helper + result, preserving actual report evidence and public single-file behavior. + Remove orphan `filecatalog.RelativePath`, `resourceNamespaceFromID`, and the + unused runner-test `stringPtr` helper with their obsolete self-tests. + +**Completion evidence:** Documentation agrees with the owning GoDoc/format/wire +contracts. Caller checks precede deletion; meaningful discovery, identity, +namespace, schema, privacy, and bundled behavior remain covered. Tests do not +merely reproduce the loader's output as their expected answer. + +## Sequencing For The Implementation Plan + +Use reviewable units with tests and owning documentation included. Workstream +letters group outcomes; they are not mandatory serial phases. + +1. Establish A's toolchain baseline for implementation validation. Independent + documentation work can proceed without waiting for that environment work. +2. Prioritize B's simplifications and C's confirmed defects. They can be split + into independent units; avoid bundling unrelated public behavior changes. +3. Sequence B's schema policy with its compilation simplification, and remove + source conversion before E's copying consolidation. Coordinate B's schema + coverage with F's helper-result removal. +4. Fix C's error/stage behavior before E's generation accounting refactor. Bound + authentication reads before or alongside relocating the protocol adapter, + with the behavioral remedy independently reviewable. +5. Coordinate D's evidence algorithm with E's traversal cleanup and D's overlays + with E's copying work. Preserve each remedy's independent evidence rather + than assuming a general refactor also fixes its associated defect. +6. Migrate the scheduling test before deleting the wrapper. Adjust over-specified + hash tests before or alongside captured-definition cleanup. Fold other small + F changes into their owning units where useful, without obscuring fixes. + +The later plan should assign concrete implementation units, validation, and +status to these outcomes. No significant product decisions remain open now. +Report representation, helper extraction boundaries, and index layout remain +ordinary implementation choices within the selected behavior. Escalate only +new evidence requiring a contract or scope change. + +## Compatibility And Documentation Delivery + +The selected ownership, schema, and convenience-source changes are deliberate +pre-1.0 compatibility changes. Deliver them in a minor release under the existing +[release policy](../policy/release.md#versioning-and-compatibility), with migration +notes explaining independent artifact storage, self-contained schemas, and +ConfigKit-supported input types. Mention changed definition hashes so consumers +can account for comparisons or caches; do not establish a stable encoding. +Release selection and publication are not part of writing this roadmap. + +Update exported GoDoc, [consumer guidance](../consumers/pkg-promptkit.md), +[formats](../formats.md), [wire integration](../integrations/openai-compatible-chat.md), +and relevant [internal documentation](../internal/overview.md) when their owning +behavior lands. Clarify ordinary development toolchain selection in +[development guidance](../development.md), linking to existing release policy. +Correct standalone documentation defects without waiting for unrelated code. + +## Validation And Completion + +Follow the [testing policy](../policy/testing.md) and existing +[maintainer validation workflow](../development.md#maintainer-validation). +Use focused behavioral regressions for demonstrated defects, race testing for +changed concurrent ownership/scheduling paths, and offline wire tests where +local success does not prove the emitted request is correct. Reuse successful +checks for unchanged content; release validation remains the existing workflow. +No live inference, provider credentials, generalized benchmark suite, or new +recurring release gate is introduced. + +The workstream catalog lists partition all 34 entries. During implementation, +record for each workstream its completed units, evidence, and any explicitly +justified deferral. A measured deferral is not an implemented fix; optional +cleanup does not hold unrelated fixes hostage. Full implementation completion +requires the selected changes, their relevant validation, migration guidance, +and removal of obsolete documentation/tests. Any remaining work must be named +and retained in an appropriate roadmap rather than lost during consolidation. + +When this roadmap's work is complete or otherwise explicitly disposed of, move +still-useful contract information to its canonical owners and retire temporary +planning documents under the [documentation lifecycle](../policy/documentation.md#document-lifecycle). +Keep audit evidence and decision provenance discoverable while they remain +useful; do not maintain competing current-behavior references.