test(signing): fix CheckInstalled signature drift in e2e skip guard - #383
Open
pierrepicaud wants to merge 1 commit into
Open
test(signing): fix CheckInstalled signature drift in e2e skip guard#383pierrepicaud wants to merge 1 commit into
pierrepicaud wants to merge 1 commit into
Conversation
The e2e build tag is never exercised in CI — branch.yaml, branch-preview.yaml and push.yaml all run a bare `go test ./...` — so this file stopped compiling unnoticed when ToolInstaller.CheckInstalled gained a context parameter and dropped its bool return. Adopt the form the sibling integration tests already use. A missing tool now surfaces as a non-nil error from exec.LookPath, so the separate installed bool it checked no longer exists. Signed-off-by: Pierre Picaud <30554557+pierrepicaud@users.noreply.github.com>
pierrepicaud
requested review from
Cre-eD,
Laboratory,
smecsia and
universe-ops
as code owners
August 17, 2026 16:45
Security Scan ResultsRepository:
Scanned at 2026-08-17 16:47 UTC |
📊 Statement coverageMeasured on the documented included set (see
Baseline: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pkg/security/signing/e2e_test.gono longer compiled under-tags e2e. ItsskipIfToolsNotInstalledhelper calledToolInstaller.CheckInstalledwith thepre-drift signature:
The call now passes a context and checks only the returned error, matching what
signing/integration_test.go:26andsecurity/executor_integration_test.go:25already do.
Closes #336.
Rationale
CheckInstalled(pkg/security/tools/installer.go:36) returns a bareerror:a failed
exec.LookPathcomes back as a non-nil error, so theinstalledboolthe test also inspected has no counterpart in the current API. Dropping it is not
a loosening of the guard: the "tool missing" case is exactly the error case.
Worth flagging separately: nothing in CI ever builds this file.
branch.yaml,branch-preview.yamlandpush.yamlall run a barego test ./..., and noworkflow passes
-tags e2eor-tags integration. That is why the drift satundetected. A cheap compile-only guard (
go test -tags e2e -run='^$' ./...)would catch the next one without needing cosign, docker, or a registry. Left out
of this PR to keep it to one concern; happy to open a follow-up issue.
Test plan
Not verified: that the e2e tests pass when actually run. That needs cosign
plus docker plus pushes to the public
ttl.shregistry, which I do not have setup. The acceptance criteria on #336 asks for a clean compile only.
Projected impact
None on the default test path:
go test ./...does not build this file beforeor after. Under
-tags e2ethe package now compiles, so the e2e suite becomesrunnable again for anyone with the tools installed.
Threat-model note
Required by CONTRIBUTING.md because the diff is under
pkg/security/. Noproduction reachability: the file is test-only and behind the
e2ebuild tag, soit ships in no binary and is excluded from every CI invocation. No entry in
SECURITY.md's STRIDE table or vectors V1-V5 is addressed or affected, and no
verification step is weakened. The skip guard still refuses to run when cosign is
absent; it just reads that absence off the error instead of a bool the API no
longer returns.