fix(os): preserve close errors in CopyFile

[zjubiology]

Overview

CopyFile has two deferred closures that are meant to return a Close() error
when the copy itself succeeded:

if cerr := srcfile.Close(); cerr != nil {
    if err == nil {
        err = cerr
    }
}

This can't work with an unnamed result. return err commits the value to the
result slot before the defers run, so err = cerr only mutates the local
variable and is discarded.

Fix is to name the result:

func CopyFile(src, dst string) (err error)

The existing defers now behave as written. Function body unchanged.

Notes:

• The function type is unchanged, so callers are unaffected.
• info, err := srcfile.Stat() and friends assign rather than shadow, since a
  named result lives in the same scope as the function body.
• nonamedreturns / nakedret aren't enabled in .golangci.yml.
• A Close() error on the read-only source will now also surface. I kept the
  original semantics since the comments state that intent, but happy to narrow
  this to the destination handle if you'd prefer.

No test added: forcing Close() to fail while io.Copy succeeds isn't
portably reproducible without threading io.WriteCloser through the internals.
Happy to do that in a follow-up if you want the path covered.

gofmt, go vet and go test ./pkg/os/... pass.

Summary by CodeRabbit

• Bug Fixes
  • File-copy operations now report errors that occur when closing the source or destination file, provided the copy itself succeeds. Errors encountered while opening, checking, creating, or copying files continue to take precedence, so the underlying cause of a failed copy is preserved.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 686e4017-2d20-4a0e-b4dd-1fe12a9e98f5

📥 Commits

Reviewing files that changed from the base of the PR and between 3f67d1b and 2ce4fd3.

📒 Files selected for processing (1)

• pkg/os/os.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

---

📝 Walkthrough

Walkthrough

CopyFile now returns a deferred close error when no earlier operation failed. Errors from earlier operations retain precedence.

Changes

CopyFile error handling

Layer / File(s)	Summary
CopyFile close error handling pkg/os/os.go	CopyFile uses a named error result. Deferred source and destination close handlers can set it when no earlier operation failed.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 2ce4f

CopyFile now reports close failures without replacing earlier errors. No actionable merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 2ce4f

The change can report a close failure after copying has completed, which may affect how callers decide to retry. No new filesystem access or privilege path is evident, but behavior of callers outside this repository is unknown.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed behavior can reach users of the exported function, but repository evidence identifies no production caller or expanded filesystem authority. External consumers are not established by the available dependency evidence.

Trust Boundaries and Controls

• observed — Source and destination selection and permission derivation remain within the existing CopyFile filesystem operation; the return change introduces no new path to a sensitive sink.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving close errors in CopyFile. It also follows the repository's semantic commit format.
Description check	✅ Passed	The description completes the required Overview section and explains the problem, rationale, implementation, compatibility impact, testing, and known limitations.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[zjubiology]

@tac0turtle Could you please take a look when you have a chance? Thanks!