fix(installer): confine tarball extraction with os.Root

[jcfs]

Summary

Triage of all 11 open code scanning alerts. One was a real, reproducible bug; the other ten were false positives or by-design and have been dismissed in the UI with per-alert reasoning.

The real one: zip-slip in tarball tool installs (CodeQL #19, high)

extractTarGz validated tar entry paths lexically only. That is escapable in two hops:

1. entry a → symlink to . — resolves to destDir itself, so it passes containment
2. entry a/x → symlink to .. — lexically "resolves" to destDir (allowed), but really lands on destDir's parent, because a is destDir
3. entry a/x/pwned.txt — an ordinary file, no .. anywhere in its name, written through the chain

Building that tarball and extracting it against the old code wrote PWNED outside destDir. Not theoretical.

The gap is that lexical validation resolves a/x as if a were a real directory. It isn't — it's a symlink, and the kernel resolves it at a shallower depth than the string math assumes.

Fix

Anchor extraction at an os.Root opened on destDir, so containment is enforced by the OS-level path walk, which refuses to follow a link out of the tree. os.Root also permits the legitimate cases we already supported (npm-style .bin/x -> ../pkg/bin/x.js relative links). The lexical checks stay in place — they now serve to surface a clear error up front rather than being the sole defense.

Requires Go 1.24+ for os.Root and 1.25+ for Root.MkdirAll/Root.Symlink; the module is already on go 1.26.0.

Reachability

Narrow — it needs a malicious or compromised asset on a pinned GitHub release that the tool installer pulls. Worth fixing regardless: the fix is small and the primitive is a full arbitrary-file-write outside the install directory.

The ten dismissed alerts

Alert(s)	Disposition
#245 go/command-injection (critical)	False positive — argv slice, no shell, flags allowlisted
#209 #204 #205 go/path-injection	Won't fix — the ADR-0016 read-only absolute-path read is intentional
#196 #189 #72 #70 #69 go/path-injection	False positive — all re-derive from a trusted root after a containment check
#6 js/request-forgery (critical)	False positive — stale Next.js-era alert; the module now runs in the browser, same-origin

Each carries its reasoning in the dismissal comment on the alert itself.

Test plan

• New regression test TestExtractTarGz_WriteThroughSymlinkChainBlocked reproduces the two-hop chain, asserts extraction errors and that nothing lands outside destDir. It fails against the pre-fix code.
• All 6 existing extraction tests still pass, including the legitimate-relative-symlink case.
• make -C apps/backend test build, golangci-lint clean.

🤖 Generated with Claude Code

[coderabbitai]

📝 Walkthrough

Walkthrough

Tarball extraction now uses os.Root for destination containment, validates symlink targets, and adds a regression test for chained symlink escapes. The Git command’s static-analysis annotation was updated without changing runtime behavior.

Changes

Tarball extraction security

Layer / File(s)	Summary
Root-anchored extraction and escape regression apps/backend/internal/tools/installer/github_tarball_strategy.go, apps/backend/internal/tools/installer/github_tarball_strategy_test.go	Extraction uses os.Root operations, rejects escaping symlink targets, preserves the file-size limit, and tests write-through symlink-chain blocking.
Command-injection annotation update apps/backend/internal/agentctl/server/process/git.go	The Git invocation now uses CodeQL-specific explanatory comments without changing execution behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: carlosflorencio

Poem

I’m a rabbit guarding files in their burrow tonight,
Symlink trails stay safely within sight.
Tarballs unpack where roots decree,
No sneaky paths escape from me.
Hop, hop—CodeQL notes shine bright!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed and relevant, but it omits the required Checklist section and includes a disallowed Summary heading and footer.	Add the Checklist section unchanged, remove the Summary heading and attribution footer, and format the body to match the template more closely.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: confining tarball extraction with os.Root.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/code-scanning-fix-j2c

---

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.}

[claude]

Claude finished @jcfs's task in 0s —— View job

---

I'll analyze this and get back to you.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b12d59655c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[claude]

Claude finished @jcfs's task in 0s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude finished @jcfs's task in 0s —— View job

---

I'll analyze this and get back to you.