218 lines
11 KiB
Markdown
218 lines
11 KiB
Markdown
|
|
# Functional correctness audit
|
||
|
|
|
||
|
|
## Status and scope
|
||
|
|
|
||
|
|
Audit completed against source commit `31f5327` on 2026-11-01. The three findings
|
||
|
|
below remain open. This audit changes documentation only.
|
||
|
|
|
||
|
|
The review covered candidate capture and rechecks, module/tag derivation,
|
||
|
|
committed-file validation and reference traversal, builds and runtime version
|
||
|
|
injection, dependency and checkout preparation, CLI adaptation and receipts,
|
||
|
|
source publication guards and recovery, and hosted page/asset delivery. It
|
||
|
|
examined ordinary paths, empty selections, error and cancellation paths, and
|
||
|
|
boundary interactions against the current code, GoDoc, consumer guides,
|
||
|
|
accepted decisions, and existing tests.
|
||
|
|
|
||
|
|
The earlier [interface audit](interfaces.md) remains applicable. In particular,
|
||
|
|
its build-source cancellation defect remains open; this document records
|
||
|
|
additional functional findings rather than duplicating that issue or its
|
||
|
|
contract-clarity findings. No production code or repository tests were changed.
|
||
|
|
|
||
|
|
## Current architecture
|
||
|
|
|
||
|
|
The root `releasekit` facade owns consumer types and coordinates checking,
|
||
|
|
building, publication, preparation, and delivery through internal packages.
|
||
|
|
The separate `buildinfo` facade exposes the runtime version accessor through a
|
||
|
|
small independent implementation.
|
||
|
|
|
||
|
|
Candidate capture, shared validation, release identity, builds, artifacts, and
|
||
|
|
Git operations have separate owners. Publication coordinates exact annotated
|
||
|
|
tag publication and recovery. Shared delivery coordinates page and asset
|
||
|
|
matching, writes, and reconciliation through the neutral `internal/forge`
|
||
|
|
contract; `internal/gitea` implements its HTTP operations. The CLI owns
|
||
|
|
profiles, token loading, receipts, CI sequencing, output, and exit statuses.
|
||
|
|
|
||
|
|
This structure remains consistent with the
|
||
|
|
[architecture policy](../policy/architecture.md). The findings require focused
|
||
|
|
behavioral corrections within those boundaries.
|
||
|
|
|
||
|
|
## Findings
|
||
|
|
|
||
|
|
### 1. Candidate checking can approve a tag that Git cannot create
|
||
|
|
|
||
|
|
**Priority:** medium. **Status:** open.
|
||
|
|
|
||
|
|
**Relevant code:** `Derive` in
|
||
|
|
[internal/releaseidentity/identity.go](../../internal/releaseidentity/identity.go),
|
||
|
|
and `Capture`, `Recheck`, and `hasLocalTag` in
|
||
|
|
[internal/candidate/check.go](../../internal/candidate/check.go). Compare the
|
||
|
|
existing `checkTagRef` boundary in
|
||
|
|
[internal/git/remote.go](../../internal/git/remote.go).
|
||
|
|
|
||
|
|
**Circumstances:** a selected nested module has a physical directory name that
|
||
|
|
produces an invalid Git ref component. For example, `module space` and
|
||
|
|
`module.lock` are usable filesystem directories but cannot occur as components
|
||
|
|
of the derived release tag.
|
||
|
|
|
||
|
|
`Derive` constructs the complete tag from the physical module path and version.
|
||
|
|
Candidate capture and rechecking use `git show-ref --verify --quiet` to test
|
||
|
|
whether that tag exists. Git exits with status 1 for these invalid names as well
|
||
|
|
as for valid absent refs. `hasLocalTag` interprets every such exit as absence,
|
||
|
|
so neither candidate guard establishes that the derived name is a usable tag.
|
||
|
|
|
||
|
|
**Expected behavior:** candidate checking should report an invalid derived tag
|
||
|
|
as a local candidate failure. An invalid ref cannot satisfy the selected Git
|
||
|
|
release-tag convention, independently of remote availability.
|
||
|
|
|
||
|
|
**Actual behavior and evidence:** a temporary public-API probe created clean
|
||
|
|
committed modules with valid declarations, Go source, and module-local notes.
|
||
|
|
With version `v1.2.3`, public `Check` returned nil errors for both
|
||
|
|
`module space/v1.2.3` and `module.lock/v1.2.3`. Independent
|
||
|
|
`git check-ref-format refs/tags/<tag>` calls rejected both. The same fixture's
|
||
|
|
ordinary `normal/v1.2.3` selection passed both checks.
|
||
|
|
|
||
|
|
**Consequences:** callers receive successful local candidate evidence for a
|
||
|
|
release identity that cannot be published. `Publish` subsequently rejects it
|
||
|
|
through its Git ref checks, so this finding is a false-positive check result,
|
||
|
|
not evidence of an invalid tag reaching the remote.
|
||
|
|
|
||
|
|
**Potential remediation:** validate the complete derived ref at the candidate
|
||
|
|
capture boundary before treating it as an absent local tag, reusing Git's
|
||
|
|
existing ref-validation rules. Keep existence and name-validity checks distinct.
|
||
|
|
Cover the public `Check` result with invalid derived names and an ordinary
|
||
|
|
nested-module control. This does not require inventing filesystem directory
|
||
|
|
restrictions or changing standalone local-build behavior.
|
||
|
|
|
||
|
|
### 2. Ambient Go flags can redirect dependency preparation to another module file
|
||
|
|
|
||
|
|
**Priority:** medium. **Status:** open.
|
||
|
|
|
||
|
|
**Relevant code:** public `PrepareBuildDependencies` in
|
||
|
|
[preparation.go](../../preparation.go), its preparation bridge, and
|
||
|
|
`PrepareBuildDependencies` in
|
||
|
|
[internal/gotool/gotool.go](../../internal/gotool/gotool.go). Binary CI delivery
|
||
|
|
calls this path from [cmd/releasekit/main.go](../../cmd/releasekit/main.go).
|
||
|
|
|
||
|
|
**Circumstances:** effective `GOFLAGS` contains `-modfile` pointing to alternate
|
||
|
|
module metadata. The value can come from the process environment or Go's
|
||
|
|
persistent environment file. This finding applies even when the caller supplies
|
||
|
|
the correct repository root and selected module, independently of the anchor
|
||
|
|
ambiguity recorded in the interface audit.
|
||
|
|
|
||
|
|
The preparation command owns `GOWORK=off` and `GOTOOLCHAIN=local` but leaves
|
||
|
|
module-selection flags inherited. Go therefore runs in the requested directory
|
||
|
|
while downloading dependencies from the alternate module file. Later validation
|
||
|
|
and artifact compilation use ReleaseKit's controlled Go flags and the selected
|
||
|
|
module's actual metadata.
|
||
|
|
|
||
|
|
**Expected behavior:** successful preparation should prepare the dependencies
|
||
|
|
of the selected module described by the request and consumed by the subsequent
|
||
|
|
ordinary build. Caller proxy, private-module, authentication, checksum, and
|
||
|
|
cache settings should remain available as documented.
|
||
|
|
|
||
|
|
**Actual behavior and evidence:** a temporary public-API probe selected a module
|
||
|
|
requiring `example.invalid/needed v1.0.0`, supplied an alternate dependency-free
|
||
|
|
module file through `GOFLAGS=-modfile=...`, and disabled module networks.
|
||
|
|
`PrepareBuildDependencies` returned nil. Removing that flag and preparing the
|
||
|
|
same selection failed with `module lookup disabled by GOPROXY=off`, confirming
|
||
|
|
that the requested graph had not been prepared. The probe used a disposable
|
||
|
|
`GOMODCACHE` for every synthetic-module download attempt.
|
||
|
|
|
||
|
|
An additional local Go probe confirmed that setting `GOFLAGS` to an empty string
|
||
|
|
does not suppress a value in a persistent Go environment file on Go 1.26.10.
|
||
|
|
A fix that only clears the process value would leave that case unresolved.
|
||
|
|
|
||
|
|
**Consequences:** preparation can report success while the selected binary build
|
||
|
|
still lacks dependencies. It can also use or update the alternate metadata's
|
||
|
|
checksum file rather than the selected module's `go.sum`, contrary to the
|
||
|
|
operator's selected preparation scope.
|
||
|
|
|
||
|
|
**Potential remediation:** give dependency preparation a controlled,
|
||
|
|
nonredirecting Go-flag baseline while preserving its network-capable environment
|
||
|
|
and effective operational settings. For example, a local Go 1.26.10 probe
|
||
|
|
confirmed that an explicit nonempty `GOFLAGS=-mod=mod` overrides a persistent
|
||
|
|
`-modfile` value and is accepted by `go mod download`; the exact baseline should
|
||
|
|
be assessed against the supported preparation behavior. Verify both inherited
|
||
|
|
and persistent flag sources, actual selected-module preparation, and retention
|
||
|
|
of the existing proxy/private-module/cache settings. Avoid globally disabling
|
||
|
|
persistent Go configuration as a shortcut, since that also discards supported
|
||
|
|
caller settings.
|
||
|
|
|
||
|
|
### 3. Incomplete tag reconciliation is reported as a known publication conflict
|
||
|
|
|
||
|
|
**Priority:** medium. **Status:** open.
|
||
|
|
|
||
|
|
**Relevant code:** `mergeRef` in
|
||
|
|
[internal/git/remote.go](../../internal/git/remote.go), `Endpoint.QueryTag` in
|
||
|
|
[internal/git/tag.go](../../internal/git/tag.go), and `reconcilePush` in
|
||
|
|
[internal/publish/publish.go](../../internal/publish/publish.go).
|
||
|
|
|
||
|
|
**Circumstances:** after a failed push acknowledgment, the authoritative tag
|
||
|
|
query succeeds as a command but supplies only the peeled
|
||
|
|
`refs/tags/<tag>^{}` record, with no exact `refs/tags/<tag>` object record. This
|
||
|
|
is an incomplete query result, not the normal output of a healthy Git remote.
|
||
|
|
Malformed and inconsistent reconciliation results already fall within the
|
||
|
|
implemented recovery scope.
|
||
|
|
|
||
|
|
`mergeRef` marks the peeled observation `RefPresent` and stores `Peeled`, while
|
||
|
|
leaving `Object` empty. `QueryTag` returns that state without final completeness
|
||
|
|
validation. `reconcilePush` does not recognize it as unreadable; it falls through
|
||
|
|
to the different-object case and reports `PublicationConflict` with a blank
|
||
|
|
observed object ID.
|
||
|
|
|
||
|
|
**Expected behavior:** the result should remain `PublicationUnknown`, preserving
|
||
|
|
the push failure and explaining that the query did not establish the exact tag
|
||
|
|
object. A peeled commit alone cannot establish equality or inequality with the
|
||
|
|
intended annotated tag object.
|
||
|
|
|
||
|
|
**Actual behavior and evidence:** a temporary public-API probe published to a
|
||
|
|
local bare repository through a Git wrapper. The wrapper performed the real
|
||
|
|
push, reported a simulated lost acknowledgment, and returned only the peeled
|
||
|
|
record during recovery. `Publish` returned `state=conflict` and
|
||
|
|
`phase=reconciliation`, with a diagnostic claiming that the remote identifies
|
||
|
|
an empty object instead of the intended tag object. An independent normal Git
|
||
|
|
query confirmed that the remote actually held the intended exact tag object.
|
||
|
|
|
||
|
|
**Consequences:** an unresolved observation is presented as a known remote
|
||
|
|
conflict. The CLI returns its known-failure status rather than the status for
|
||
|
|
an uncertain publication, and callers receive misleading recovery evidence
|
||
|
|
even though the push may have succeeded.
|
||
|
|
|
||
|
|
**Potential remediation:** finalize ref observations after parsing the complete
|
||
|
|
response and classify a peeled-only result as inconsistent or unreadable.
|
||
|
|
Preserve valid responses where a peeled record precedes its exact object record.
|
||
|
|
Keep recovery unknown whenever the exact object cannot be determined. A focused
|
||
|
|
recovery regression should retain the original push cause, verify the public
|
||
|
|
unknown state, and preserve ordinary exact-match, absence, and real-conflict
|
||
|
|
outcomes.
|
||
|
|
|
||
|
|
## Validation and limits
|
||
|
|
|
||
|
|
The ordinary and race-enabled offline suites and compiled API/dependency-boundary
|
||
|
|
check passed with Go 1.26.10 on Linux/amd64:
|
||
|
|
|
||
|
|
```sh
|
||
|
|
GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test ./...
|
||
|
|
GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test -race ./...
|
||
|
|
GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go run ./internal/apicheck
|
||
|
|
```
|
||
|
|
|
||
|
|
The probes used temporary programs outside the repository, disposable local
|
||
|
|
Git repositories, command wrappers, and offline Go invocations. They exercised
|
||
|
|
public APIs and verified the relevant external facts independently: Git ref
|
||
|
|
validity, dependency preparation against actual module metadata, and the exact
|
||
|
|
object present on a local remote. No live forge, CI runner, or real credentials
|
||
|
|
were involved.
|
||
|
|
|
||
|
|
The audit also reviewed committed-file input boundaries, local-reference and
|
||
|
|
symlink traversal, validation ordering, build result/receipt binding, version
|
||
|
|
injection, and additive delivery with read-only preflight and write recovery.
|
||
|
|
No additional functional finding was substantiated in those paths beyond the
|
||
|
|
issues already recorded in [the interface audit](interfaces.md). Passing tests
|
||
|
|
provide regression evidence for existing cases; they do not resolve the three
|
||
|
|
newly demonstrated cases above.
|
||
|
|
|
||
|
|
Prioritize accurate publication recovery and preparation of the selected module,
|
||
|
|
then close the candidate tag-validation gap. These are corrections to existing
|
||
|
|
behavior; they do not require new public interfaces or a broader release model.
|