Add audit remediation roadmap
This commit is contained in:
366
docs/roadmap/audit-remediation.md
Normal file
366
docs/roadmap/audit-remediation.md
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user