From 34d7a19da52bd68fe5eea19df069db8870c08e52 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 11 Aug 2026 14:17:55 +0000 Subject: [PATCH] Record public value and error audit findings --- audit.md | 134 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 audit.md diff --git a/audit.md b/audit.md new file mode 100644 index 0000000..41c3cee --- /dev/null +++ b/audit.md @@ -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.