[jumboduck]

kosli get rego-policy gets a --download FILE flag. With --policy-version, it writes the bundle of that version to FILE as a gzipped tar, exactly as the server sends it. It uses the bundle route that kosli-dev/server#7590 added. Part of kosli-dev/server#7380.

kosli get rego-policy pr-approved \
	--policy-version 3 \
	--download ./pr-approved-v3.tgz

Behaviour

• It prints one line, for example Downloaded version 3 of rego policy 'pr-approved' to ./pr-approved-v3.tgz, in place of the version table.
• --download without --policy-version is refused before any request. An empty --download is refused too.
• An existing FILE is refused and kept. The error says to remove it or choose another file.
• A 404 shows the server's message, and no file is created.
• The bundle is not extracted. An earlier revision of this PR extracted it into a directory; that was dropped after review, and can follow as its own slice if users ask for it. To edit and republish, run tar -xzf first.

Tests

• Unit test for BundleURL.
• Command tests: the download, the refusal to overwrite, the refusal without --policy-version, the empty value, and the 404.
• The empty-flag audit coverage and spec include download.
• make lint: 0 issues. Not yet run end to end against a server with the bundle route.

Checklist

• Docs are autogenerated from CLI help — make sure the help text reflects your changes
• Helm chart (charts/k8s-reporter/) updated, if needed. Note: these changes live in a separate PR
• Terraform provider and related changes (terraform-provider-kosli, terraform-aws-evidence-reporter, terraform-aws-kosli-reporter) updated, if needed

🤖 Generated with Claude Code

[claude]

Claude finished @jumboduck's task in 3m 6s —— View job

---

Follow-up review of the newest commits (1a3bf266, 2333f995)

• Gather context (new commits, previous findings)
• Evaluate status of each previous finding touched by new commits
• Review new commits' changed lines for new issues
• Post inline comments for resolved/new findings — none needed, see below
• Post summary comment

Note: BEFORE=0fcec9e9 wasn't reachable locally (the branch was rebased — df6f38ae/0fcec9e9 now have new hashes 1a3bf266/2333f995), so per the fallback rule I used gh pr diff / gh api compare, confirming via git log that these two commits are exactly the ones added since the last review round.

Status of previously raised findings touched by these commits:

• SnykCode path traversal, duplicate/colliding archive members, symlink escape, missing-ancestor-directory cleanup (all in internal/regopolicies/bundle.go) — resolved/moot, unchanged from the prior round: that file and its tests are gone, deleted in 1a3bf266 (df6f38ae pre-rebase). All corresponding review threads are already marked resolved on GitHub.
• "Write failure leaves a stuck partial file with a misleading 'already exists' error" (cmd/kosli/getRegoPolicy.go, writeNewFile) — resolved by 2333f995 (0fcec9e9 pre-rebase). Verified in the current code (cmd/kosli/getRegoPolicy.go:157-174): writeNewFile now calls file.Close() unconditionally, keeps the first error from Write/Close, and os.Remove(path) runs whenever that combined error is non-nil — so a failed download no longer blocks a clean retry. The review thread is already marked resolved.

New issues in the changed lines: none found. I checked the diff 1a3bf266..2333f995 (doc text, runDownload/writeNewFile, the updated command tests, root.go flag text, and the bundle.go/bundle_test.go deletions) and confirmed no leftover references to the removed ExtractBundle/bundleFile types anywhere in the tree. Logic in writeNewFile is correct: a failure during OpenFile with ErrExist returns early without touching the filesystem; any other OpenFile failure returns before a file exists; a Write/Close failure after a file was created is now cleaned up.

Could not run go build/make lint/tests in this session due to tool permission restrictions (network/build commands required approval that wasn't available) — verification here is by code reading only.

0 new inline comments posted.

[claude]

Follow-up review of the new commits (002f85d5..df6f38a)

The approach changed substantially: ef8d15f9/002f85d5's directory-extraction code (internal/regopolicies/bundle.go + its tests) was deleted entirely. df6f38ae reimplements --download as pbeckham suggested — it now writes the server's gzipped-tar bundle unchanged to a single file opened with O_EXCL, with no tar parsing at all.

Status of previously raised findings:

• SnykCode path traversal, duplicate/colliding archive members, symlink escape out of DIR, missing-ancestor-directory cleanup (mine), upload-cap/size-bound tracking, and the utils.LocalRelativePath suggestion — all resolved/moot: the file they applied to is gone, since the command no longer parses or extracts an archive. I replied on and resolved the one thread that was still open (ancestor-directory cleanup).
• Empty --download "" refusal — still resolved via the existing root-command flag-wrapping guard; untouched by this diff and still pinned by a test.
• "No --force, re-download always fails" (pbeckham) — still open/unaddressed, same behavior as before but now for a single file (already exists; remove it or choose another file). Acknowledged by the author as a possible future slice, not required here.
• "Do we need to extract in this PR? A thinner --download FILE slice..." (pbeckham) — adopted: this is exactly what df6f38ae implements.

New finding in the changed lines (posted inline): writeNewFile (cmd/kosli/getRegoPolicy.go:157-169) doesn't clean up the file if file.Write fails partway — a failed download leaves a partial file behind, and retrying then hits the "already exists" error with no indication the existing file is actually incomplete/corrupt.

View job run