Record public value and error audit findings
This commit is contained in:
134
audit.md
Normal file
134
audit.md
Normal file
@@ -0,0 +1,134 @@
|
||||
# Codebase Audit
|
||||
|
||||
## Sequence Prerequisite
|
||||
|
||||
Stage 0 had not been executed when this artifact was created: the repository
|
||||
contained no `audit.md` at the start of the Stage 1 review. Consequently, the
|
||||
reproducible repository baseline, package inventory, validation matrix, graph
|
||||
refresh, and initial coverage ledger required by Stage 0 remain outstanding.
|
||||
This review did not backfill that out-of-scope work.
|
||||
|
||||
The Stage 1 review began from commit
|
||||
`ebf1602635e108e2a7ac1abd3a3ca24a620104ce` on branch `main`, with a clean
|
||||
working tree and Go `go1.26.5 linux/amd64`. These details identify this review
|
||||
only; they are not a substitute for the Stage 0 baseline.
|
||||
|
||||
## Stage 1: Public Values, Conversion, Errors, And Formatting
|
||||
|
||||
### Scope Reviewed
|
||||
|
||||
The review covered `doc.go`, `types.go`, `convert.go`, `errors.go`,
|
||||
`capacity_error.go`, `formatting.go`, and `prepared_execution.go`, plus the
|
||||
directly relevant root tests. Internal domain declarations and callers were
|
||||
consulted only to confirm field-complete conversion, ownership, and public
|
||||
error mapping. Engine assembly, runtime orchestration, adapter implementation,
|
||||
and JSON codec mechanics were not audited.
|
||||
|
||||
### Accepted Findings
|
||||
|
||||
#### S01-F01: Run-request formatting tests do not protect input and variable redaction
|
||||
|
||||
- **Category:** testing
|
||||
- **Severity:** medium
|
||||
- **Confidence:** high
|
||||
- **Status:** accepted
|
||||
- **Affected code:** `formatting.go` (`RunRequest.String`,
|
||||
`RunRequest.GoString`, and `RunRequest.redactedString`) and `engine_test.go`
|
||||
(`TestRunRequestFormattingRedactsDirectAPIKey`)
|
||||
- **Contract at issue:** The formatter GoDoc promises that `String` and
|
||||
`GoString` omit direct credentials and input and variable contents. The
|
||||
documentation and testing policies treat prompt inputs and other private
|
||||
content as sensitive and give consequential disclosure behavior a strong
|
||||
presumption of durable test protection.
|
||||
- **Evidence:** The implementation currently satisfies the contract by
|
||||
formatting only request identifiers, collection lengths, presence flags,
|
||||
and whether an API key is set. The focused test supplies an inline input but
|
||||
asserts only that the API-key sentinel is absent and `APIKeySet:true` is
|
||||
present; it supplies no variable values and never checks whether the input
|
||||
URI, input body, or variable values appear. The test would remain green if a
|
||||
later formatter change appended input or variable contents while continuing
|
||||
to omit the API key.
|
||||
- **Failure mode:** A logging or diagnostic-formatting refactor could disclose
|
||||
prompt input or template-variable content through ordinary `%v`, `%+v`, or
|
||||
`%#v` formatting without a contract-test failure.
|
||||
- **Recommended direction:** Extend the existing formatting test, rather than
|
||||
adding a parallel test, with distinct input URI, input body, and variable
|
||||
sentinels and assert that each is absent from all three supported formatting
|
||||
forms. Retain the positive structural assertions so the test continues to
|
||||
distinguish a useful summary from an empty formatter.
|
||||
- **Required verification:** Run the focused request-formatting test and the
|
||||
root package tests. Confirm that a deliberate formatter mutation exposing
|
||||
any sentinel makes the focused test fail.
|
||||
|
||||
### Unresolved Observations
|
||||
|
||||
None. Medium- or low-confidence concerns discovered during this review were
|
||||
not promoted to findings.
|
||||
|
||||
### Coverage Ledger
|
||||
|
||||
- **Public value declarations and zero values:** Reviewed request, prepared,
|
||||
result, artifact, inspection, target, output, validation, rendered-prompt,
|
||||
structured-output, generation, and token-usage values. Nil maps, slices,
|
||||
pointers, optional values, and zero-value public enum strings cross the
|
||||
facade without panics or invented values.
|
||||
- **Request conversion:** `toDomainRunRequest` and its helpers preserve every
|
||||
public field, copy maps and pointer values, validate and deeply copy nested
|
||||
JSON-compatible overrides, and retain direct credentials only in the
|
||||
internal request field intended for execution.
|
||||
- **Prepared and result conversion:** `fromDomainPreparedRun`,
|
||||
`fromDomainRunResult`, and their helpers preserve all public fields while
|
||||
copying artifact bytes, validation diagnostics, hashes, rendered messages,
|
||||
cache-control pointers, effective extra parameters, and structured-output
|
||||
schemas. Internal credential and target-presence fields do not escape.
|
||||
- **Inspection and extension conversion:** Profile and prompt inspection
|
||||
outputs are independent copies. Generation requests receive copied prompt,
|
||||
target, presence, and structured-output values; generation responses contain
|
||||
no mutable fields requiring additional copying.
|
||||
- **Copy-rule ownership:** Caller-supplied JSON-compatible values enter through
|
||||
`internal/jsonvalue` validation and copying. The outward conversion helpers
|
||||
copy already-validated domain snapshots without introducing a second
|
||||
acceptance policy. No consolidation finding was warranted in this scope.
|
||||
- **Public errors:** Not-found identities remain distinct from load failures;
|
||||
profile-required and missing-credential errors retain their more specific
|
||||
identity together with `ErrInvalidRequest`; collaborator and cancellation
|
||||
identities remain discoverable; and typed capacity errors expose only a
|
||||
copied backend ID plus `ErrCapacityExceeded` rather than the internal error
|
||||
type. Nil and zero `CapacityError` values are safe.
|
||||
- **Diagnostic formatting:** `RunRequest` and `GenerateRequest` currently omit
|
||||
direct credentials and content from `String`, `GoString`, `%+v`, and `%#v`.
|
||||
`PreparedExecution` always formats as an opaque constant, including through
|
||||
a copied handle. Prepared and result values intentionally expose rendered or
|
||||
generated content as documented in package GoDoc; applications retain
|
||||
responsibility for logging those content-bearing values.
|
||||
- **Prepared handle values:** Nil and zero handles return zero details and may
|
||||
be discarded safely. Details are fresh deep copies and remain stable after
|
||||
execution or discard. Copying a handle shares its single-use lifecycle
|
||||
without exposing the internal representation.
|
||||
- **Test ownership:** Root external-package tests appropriately own public
|
||||
snapshots, structured error identity, and opaque-handle behavior. Focused
|
||||
internal error-mapping tests cover internal-type containment. The one
|
||||
material redaction gap is recorded as S01-F01.
|
||||
|
||||
### Verification Performed
|
||||
|
||||
The code knowledge graph was used to discover the scoped symbols, trace their
|
||||
callers and callees through the facade and internal domain boundary, and locate
|
||||
the focused tests. Important conclusions were confirmed against source.
|
||||
|
||||
The following focused commands passed:
|
||||
|
||||
```sh
|
||||
go test . -run 'Test(PreparedRunJSONDoesNotExposeSecretOrTargetPresence|RunRequestFormattingRedactsDirectAPIKey|GenerateRequestFormattingRedactsDirectAPIKey|MapPublicErrorPreservesGenerationCancellation|MapPublicErrorTranslatesCapacityError|CapacityExceededSentinelContract|InspectProfileReturnsIndependentTargetMatchingPreparation|InspectPromptReturnsIndependentMetadataMatchingPreparation|PreparedExecutionFreezesSourcesAndReturnsIndependentDetails|PreparedExecutionLifecycleAndEngineBinding|PreparedExecutionDiscardAndFormattingDoNotExposePrivateState|InMemoryProfileExtraParamsAreCopiedAcrossPublicBoundary|ExtraParamsTypedNestedValuesAreCopiedAcrossPublicBoundary|ArtifactReaderReceivesPublicReferenceAndPreparesArtifact|RunPassesPreparedRequestToInjectedLLMClient|PublicErrorsSupportErrorsIs)$'
|
||||
go test -race . -run 'Test(PreparedExecutionFreezesSourcesAndReturnsIndependentDetails|PreparedExecutionConcurrentClaimAllowsOneGeneration|PreparedExecutionRunAndDiscardRaceHasOneWinner|PreparedExecutionDiscardAndFormattingDoNotExposePrivateState|MapPublicErrorTranslatesCapacityError|RunRequestFormattingRedactsDirectAPIKey|GenerateRequestFormattingRedactsDirectAPIKey)$' -count=3
|
||||
```
|
||||
|
||||
### Handoff
|
||||
|
||||
- Execute the missing Stage 0 baseline before relying on this file as a
|
||||
complete audit ledger or beginning the next component review.
|
||||
- Stage 2 owns public configuration helpers, `json.go`, extension-adapter
|
||||
implementation, and adapter-specific mutation and cancellation behavior.
|
||||
- Stages 3 and 4 own engine construction and runtime operations respectively;
|
||||
this review did not evaluate those paths beyond tracing their use of the
|
||||
scoped conversion and error boundary.
|
||||
Reference in New Issue
Block a user