You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The GitSSHSignatureRequest.resolveKeyFile() method — which writes a provided public key to a temp file and returns a cleanup function — had no test coverage of its temp-file lifecycle. The existing server_test.go only covered Sign() failure paths (non-existent key, public-key content error message). Specifically, the following were untested:
Temp file is created with the correct public-key content when PublicKey is set.
The returned cleanup function actually removes the temp file.
When PublicKey is empty, resolveKeyFile returns the original KeyPath and cleanup is a no-op (does not delete the caller's key file).
The last gap is a correctness-sensitive edge case: a buggy cleanup that removed the original key file would silently delete user credentials.
Change
Added three focused tests to pkg/gitsshsigning/server_test.go (no production code changed — test-only):
TestResolveKeyFile_PublicKey_CreatesAndCleansTempFile — verifies a temp file is created, its content equals the provided public key, and the resolved path differs from the original KeyPath.
TestResolveKeyFile_PublicKey_CleanupRemovesTempFile — verifies the returned cleanup removes the temp file.
TestResolveKeyFile_EmptyPublicKey_ReturnsKeyPath — verifies the empty-PublicKey fallback returns KeyPath unchanged and cleanup does not remove the original key file.
No behavioral change; no secrets or key material are printed (only synthetic test keys are used).
Verification performed
task cli:format — clean.
task cli:lint:ci — 0 issues (gosec G304 on the test-controlled temp-file read was suppressed with a //nolint:gosec directive; errcheck satisfied via defer func() { _ = f.Close() }()).
Known pre-existing failures (not caused by this change): pkg/gitTestRepoClone* (stale assertion on origin/main) and pkg/dockerTestRunCmd_CancelKillsProcessGroup (environment process-group flake, confirmed failing on clean origin/main with changes stashed). Neither package is touched by this PR.
This PR was created by an AI agent as part of an automated daily package review job.
Add tests for GitSSHSignatureRequest.resolveKeyFile covering the public-key temp-file creation, content verification, cleanup-on-close, and the empty-public-key fallback to KeyPath.
The existing tests only covered Sign() failure paths; resolveKeyFile had no coverage of its temp-file lifecycle or the no-op cleanup branch.
AI Reviewer: run a review on demand. To trigger the first review automatically, go to your organization or repository integration settings. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
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
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.
Package reviewed
pkg/gitsshsigning(SSH/git signing category)Issue found
The
GitSSHSignatureRequest.resolveKeyFile()method — which writes a provided public key to a temp file and returns a cleanup function — had no test coverage of its temp-file lifecycle. The existingserver_test.goonly coveredSign()failure paths (non-existent key, public-key content error message). Specifically, the following were untested:PublicKeyis set.cleanupfunction actually removes the temp file.PublicKeyis empty,resolveKeyFilereturns the originalKeyPathand cleanup is a no-op (does not delete the caller's key file).The last gap is a correctness-sensitive edge case: a buggy cleanup that removed the original key file would silently delete user credentials.
Change
Added three focused tests to
pkg/gitsshsigning/server_test.go(no production code changed — test-only):TestResolveKeyFile_PublicKey_CreatesAndCleansTempFile— verifies a temp file is created, its content equals the provided public key, and the resolved path differs from the originalKeyPath.TestResolveKeyFile_PublicKey_CleanupRemovesTempFile— verifies the returnedcleanupremoves the temp file.TestResolveKeyFile_EmptyPublicKey_ReturnsKeyPath— verifies the empty-PublicKeyfallback returnsKeyPathunchanged and cleanup does not remove the original key file.No behavioral change; no secrets or key material are printed (only synthetic test keys are used).
Verification performed
task cli:format— clean.task cli:lint:ci— 0 issues (gosec G304 on the test-controlled temp-file read was suppressed with a//nolint:gosecdirective; errcheck satisfied viadefer func() { _ = f.Close() }()).task cli:test—pkg/gitsshsigningpasses (ok github.com/devsy-org/devsy/pkg/gitsshsigning).pkg/gitTestRepoClone*(stale assertion on origin/main) andpkg/dockerTestRunCmd_CancelKillsProcessGroup(environment process-group flake, confirmed failing on cleanorigin/mainwith changes stashed). Neither package is touched by this PR.This PR was created by an AI agent as part of an automated daily package review job.