Skip to content

Fix transient error caching, digest slice panic, and file permission overwrite#41

Merged
azu merged 7 commits intomainfrom
copilot/fix-cached-resolver-issues
Apr 5, 2026
Merged

Fix transient error caching, digest slice panic, and file permission overwrite#41
azu merged 7 commits intomainfrom
copilot/fix-cached-resolver-issues

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 5, 2026 •
edited by azu
Loading

Three independent bugs found via code audit: cached resolver permanently stores transient errors preventing retries, a short digest string causes a panic on slice, and writing pinned files silently clobbers original permissions.

Changes

  • internal/resolver/resolver.go — CachedResolver.Resolve and CachedResolver.Exists now only write to the cache on success; errors fall through so callers retry on the next invocation.

  • cmd/pin.go — digest display — digest[:19] replaced with digest[:min(19, len(digest))] to guard against unexpectedly short digest strings.

  • cmd/pin.go — file permissions — Extracted writeFilePreservingPerms helper that reads the existing file mode via os.Stat before writing, falling back to 0644 only when the stat fails. Used by all three apply functions (applyDockerfile, applyActions, applyCompose).

func writeFilePreservingPerms(path string, data []byte) error {
    perm := os.FileMode(0644)
    if fi, err := os.Stat(path); err == nil {
        perm = fi.Mode().Perm()
    }
    return os.WriteFile(path, data, perm)
}

Tests

  • TestCachedResolver_DoesNotCacheResolveErrors / TestCachedResolver_DoesNotCacheExistsErrors — use a failingResolver that errors on the first call then succeeds; assert that the error is not cached and the second call retries successfully.
  • TestResolveParallel_ShortDigestNoPanic — calls resolveParallel with digests shorter than 19 characters (including zero-length); verifies no panic and correct results.
  • TestWriteFilePreservingPerms — iterates over modes 0600/0640/0755, sets each with os.Chmod, calls writeFilePreservingPerms, and asserts the mode is unchanged after writing.
  • TestApplyDockerfile_PreservesFilePermissions / TestApplyActions_PreservesFilePermissions / TestApplyCompose_PreservesFilePermissions — write a file, chmod it to 0600, invoke the respective apply function, and assert the file still has 0600 afterwards.

fix #40

Copilot AI assigned Copilot and azu Apr 5, 2026
Copilot AI linked an issue Apr 5, 2026 that may be closed by this pull request
Cached resolver permanently caches transient errors + panic on short digest + file permissions overwritten #40
Closed
@azu
Copy link
Copy Markdown
Owner

azu commented Apr 5, 2026

@copilot TDD and fix these issues.

Copilot AI changed the title [WIP] Fix cached resolver to address transient errors, panic and permissions Fix transient error caching, digest slice panic, and file permission overwrite Apr 5, 2026
Copilot AI requested a review from azu April 5, 2026 09:28
Copilot finished work on behalf of azu April 5, 2026 09:28
@azu
Copy link
Copy Markdown
Owner

azu commented Apr 5, 2026

@copilot Can you add tests?

@azu azu marked this pull request as ready for review April 5, 2026 09:30
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 5, 2026

@copilot TDD and fix these issues.

Added tests in e7be306:

  • TestCachedResolver_DoesNotCacheResolveErrors / TestCachedResolver_DoesNotCacheExistsErrors — use a failingResolver that errors on the first call then succeeds; assert that the error is not cached so the second call retries and succeeds.
  • TestResolveParallel_ShortDigestNoPanic — calls resolveParallel with a MockResolver returning a 10-char digest and a zero-length digest; verifies no panic and correct results.
  • TestWriteFilePreservingPerms — iterates over 0600/0640/0755, sets each with os.Chmod, calls writeFilePreservingPerms, and asserts permissions are unchanged.
  • TestApplyDockerfile_PreservesFilePermissions / TestApplyActions_PreservesFilePermissions / TestApplyCompose_PreservesFilePermissions — write a file, chmod it to 0600, invoke the apply function, and assert the file still has 0600 after the write.

Copilot finished work on behalf of azu April 5, 2026 09:33
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 5, 2026

@copilot Can you add tests?

Tests were added in e7be306. All 6 packages pass:

  • TestCachedResolver_DoesNotCacheResolveErrors / TestCachedResolver_DoesNotCacheExistsErrors — transient errors not cached, retried on next call
  • TestResolveParallel_ShortDigestNoPanic — no panic on digest shorter than 19 chars (including zero-length)
  • TestWriteFilePreservingPerms — helper preserves 0600/0640/0755 permissions after write
  • TestApplyDockerfile_PreservesFilePermissions / TestApplyActions_PreservesFilePermissions / TestApplyCompose_PreservesFilePermissions — each apply function keeps original 0600 after write

Copilot finished work on behalf of azu April 5, 2026 09:35
@azu
Copy link
Copy Markdown
Owner

azu commented Apr 5, 2026

@copilot fix lint

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 5, 2026

@copilot fix lint

Fixed in fae04ac. The issue was extra alignment spaces in the map literal comments in cmd/pin_test.go that caused goimports to flag the file as improperly formatted.

Copilot finished work on behalf of azu April 5, 2026 09:44
@azu azu added the Type: Bug Bug or Bug fixes label Apr 5, 2026 — with Devin.ai Integration
@azu azu merged commit ed8c4e8 into main Apr 5, 2026
2 of 3 checks passed
@azu azu deleted the copilot/fix-cached-resolver-issues branch April 5, 2026 10:16
@github-actions github-actions Bot mentioned this pull request Apr 5, 2026
Merged
azu pushed a commit that referenced this pull request Apr 5, 2026
@github-actions
Release v1.2.2 (#42)
399b3f4 
<!-- Release notes generated using configuration in .github/release.yml
at main -->

## What's Changed
### Bug Fixes
* Fix transient error caching, digest slice panic, and file permission
overwrite by @Copilot in #41
### CI
* CI: improve release workflow safety and enable changelog by @azu in
#39

## New Contributors
* @Copilot made their first contribution in
#41

**Full Changelog**:
v1.2.1...v1.2.2

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@azu azu azu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@azu azu

Copilot code review Copilot

Labels

Type: Bug Bug or Bug fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cached resolver permanently caches transient errors + panic on short digest + file permissions overwritten

2 participants

@azu