Skip to content

feat(run): auto-resolve Linux binary for cross-platform security scan - #662

Merged
waynesun09 merged 3 commits into
mainfrom
fix-cross-platform-security-scan
May 6, 2026
Merged

feat(run): auto-resolve Linux binary for cross-platform security scan#662
waynesun09 merged 3 commits into
mainfrom
fix-cross-platform-security-scan

Conversation

@waynesun09

@waynesun09 waynesun09 commented May 5, 2026

Copy link
Copy Markdown
Member

Summary

  • When running fullsend run on macOS with a Linux sandbox, the pre-agent security scan (fullsend scan context) fails because the host Mach-O binary cannot execute inside the Linux container
  • Add automatic Linux binary resolution with a 4-priority fallback chain: explicit --fullsend-binary, download from matching GitHub Release, cross-compile from source, download latest release
  • Harden the download/extraction path: path traversal protection in tar extraction, io.LimitReader on compressed stream (200 MB) and extracted binary (500 MB), HTTP client with 120s timeout, ELF validation (OS/ABI + architecture), FULLSEND_SANDBOX_ARCH whitelist, GOMOD=/dev/null handling
  • Update docs/guides/dev/local-dev.md with binary resolution docs and FULLSEND_SANDBOX_ARCH usage

A companion user-facing guide (docs/guides/user/running-agents-locally.md) is split into a separate PR: #663.

Test plan

  • go test -short ./internal/cli/ — all tests pass
  • go test -short ./... — full suite passes, no regressions
  • go vet ./internal/cli/ — clean
  • make lint — clean
  • Verified locally: fullsend run review on macOS/arm64 with podman Linux sandbox auto-downloads Linux binary and completes security scan
  • Verified --fullsend-binary with explicit cross-compiled binary
  • Verified macOS Mach-O binary is rejected with clear error
  • CI: confirm no behavior change on Linux (uses os.Executable() directly)

@fullsend-ai-review

fullsend-ai-review Bot commented May 5, 2026

Copy link
Copy Markdown

Review: #662

Head SHA: 287c431
Timestamp: 2026-05-05T00:00:00Z
Outcome: comment-only

Summary

This PR adds a well-structured cross-platform binary resolution system for running security scans inside Linux sandboxes from macOS hosts. The security hardening is solid: SHA256 checksum verification on downloads, path traversal protection in tar extraction, size limits on both compressed and extracted data, ELF validation, and a strict architecture allowlist. Two medium-severity findings are worth addressing but neither blocks merge: (1) when binary resolution fails in open mode, the sandbox-side scan is attempted but fails with confusing warnings rather than being explicitly skipped, and (2) on Linux hosts with FULLSEND_SANDBOX_ARCH set to a different architecture, needsCrossCompilation() returns false but validateLinuxBinary rejects the host binary, blocking execution without triggering the cross-compile fallback.

Findings

Critical

None.

High

None.

Medium

  • [correctness] internal/cli/run.go:601-607 — When resolveLinuxBinary fails in open mode, localBinary is set to "" and no binary is copied to the sandbox. However, the sandbox-side security scan (Path B, ~line 367) still runs unconditionally when h.SecurityEnabled() is true, attempting to execute a nonexistent fullsend binary. This produces confusing "Security scan SSH failed" warnings. The host-side scan (Path A) still runs, so security coverage is not eliminated, but the degradation is implicit rather than explicit.
    Remediation: When localBinary is empty after the resolution block, set a flag (e.g., skipSandboxScan) and use it to skip the sandbox-side scan with an explicit log message like "Skipping sandbox-side security scan: no Linux binary available."

  • [correctness] internal/cli/run.go:~1280,1295needsCrossCompilation() only checks runtime.GOOS != "linux". On a Linux/amd64 host with FULLSEND_SANDBOX_ARCH=arm64, the function returns false, so os.Executable() is used. Then validateLinuxBinary rejects it because the ELF machine type doesn't match arm64. The cross-compile/download fallback is never attempted, and the user gets a hard error. The old code would have at least copied the binary (failing at runtime); the new code fails earlier with a clear error, which is better, but doesn't attempt the available fallback strategies.
    Remediation: Expand the cross-compilation trigger to also fire when sandboxArch() != runtime.GOARCH, e.g. func needsCrossCompilation() bool { return runtime.GOOS != "linux" || sandboxArch() != runtime.GOARCH }.

Low

  • [style] internal/cli/run.go — The ~310 lines of new functions (binary resolution, download, extraction, cross-compilation) are added at the bottom of an already large file (~1240 lines). Consider extracting these into a dedicated file (e.g., binary_resolve.go) to keep run.go focused on the run command logic.

Info

  • [security] Download and extraction security is well-implemented: SHA256 checksum verification before extraction, io.LimitReader on both compressed (200 MB) and extracted (500 MB) streams, path traversal protection in tar extraction, ELF OS/ABI + architecture validation, and FULLSEND_SANDBOX_ARCH validated against a fixed allowlist. No injection paths exist in URL construction (version restricted to digits/dots by isReleasedVersion, arch restricted to allowlist).

  • [correctness] Test coverage is thorough: path traversal rejection, valid extraction, checksum match/mismatch, invalid hex hash, architecture fallback with malicious input, cross-compilation (non-short mode), version parsing, and sandbox arch override. The httptest.Server approach for testing download functions with releaseBaseURL override is clean.

  • [intent-alignment] No linked issues. The PR title says "feat" which accurately describes the change — this adds new capability (cross-platform binary resolution) rather than fixing existing behavior. The scope is well-contained to the run command's sandbox bootstrap path and documentation.

Footer

Outcome: comment-only
This review applies to SHA 287c431203f248f28e4cb4f67639c3c16596af15. Any push to the PR head clears this review and requires a new evaluation.

Previous run

Review: #662

Head SHA: b30573e
Timestamp: 2026-05-05T00:00:00Z
Outcome: comment-only

Summary

This PR adds a well-structured cross-platform binary resolution system for the pre-agent security scan. The implementation is solid: path traversal protection in tar extraction, size limits via io.LimitReader, ELF validation, architecture whitelisting, and safe command construction via exec.Command. The 4-priority fallback chain (explicit binary → release download → cross-compile → latest release) is cleanly implemented. Test coverage is good, including path traversal and version parsing edge cases. Three non-blocking findings are worth considering for hardening.

Findings

Medium

  • [platform-security] internal/cli/run.go:1418-1444 — Downloaded release binaries are not verified against checksums or signatures. GoReleaser generates checksums.txt alongside release assets. An attacker who compromises a GitHub release or intercepts the download could supply a malicious binary that executes inside the sandbox with full access to the repo and credentials. Consider downloading and verifying the SHA256 checksum file.
    Remediation: Download checksums.txt from the same release, verify the archive SHA256 before extraction.

Low

  • [correctness] internal/cli/run.go:597-603 — When all binary resolution strategies fail, the security scan is silently skipped (fail-open). For harnesses with fail_mode: closed, this should be a hard error rather than a warning, since the security scan is a critical gate.
    Remediation: Check h.FailModeClosed() and return an error instead of continuing without the scan binary.

Info

  • [correctness] internal/cli/run.go:1450-1463resolveLatestReleaseTag() hits the unauthenticated GitHub API (60 req/hr rate limit). In CI environments or repeated local runs, this can fail silently. Consider using GH_TOKEN from the environment to authenticate the request if available.

Footer

Outcome: comment-only
This review applies to SHA b30573eaae0dfbf07ff20c26d5217b56d58b1908. Any push to the PR head clears this review and requires a new evaluation.

Previous run (2)

Review: #662

Head SHA: c105b7f
Timestamp: 2026-05-05T00:00:00Z
Outcome: comment-only

Summary

Well-structured feature that solves a real cross-platform problem with solid security hardening (path traversal protection, size limits, ELF validation, arch whitelist). The fallback chain is sensible and well-documented. Three non-blocking observations: the binary-resolution failure path doesn't check FailModeClosed() before continuing (relying instead on the downstream scan failure to enforce it, which produces confusing error messages), the GitHub API fallback is unauthenticated, and validateLinuxBinary is now applied to the host binary on Linux (transparent but new behavior).

Findings

Medium

  • [correctness] internal/cli/run.go:595-601 — When resolveLinuxBinary fails on macOS, the code warns on stderr and sets localBinary = "", skipping the binary copy. The subsequent Path B security scan (fullsend scan context, line 367–389 in the caller) will then fail with a confusing "command not found" error rather than a clear message about the missing binary. More importantly, the FailModeClosed() check is deferred to the scan failure handler rather than being checked at the point of resolution failure. While fail-closed mode is ultimately respected (the scan failure triggers it), the error flow is noisy and the user sees two warnings plus a cryptic scan failure instead of one clear error.
    Remediation: Check h.FailModeClosed() immediately when resolveLinuxBinary fails and return a descriptive error (e.g., "fail_mode: closed requires a Linux binary for sandbox security scan; use --fullsend-binary or install Go toolchain"). In fail-open mode, the current warning-and-continue behavior is fine.

Low

  • [correctness] internal/cli/run.go:1464-1475resolveLatestReleaseTag() calls the GitHub API without authentication. Unauthenticated requests are rate-limited to 60/hour. In CI environments or rapid iteration, this fallback could fail silently. Consider using the GITHUB_TOKEN environment variable (if available) to add an Authorization header, or at minimum document the rate-limit risk in the error message.

Info

  • [correctness] internal/cli/run.go:608-610 — On Linux hosts, validateLinuxBinary is now called on the result of os.Executable(). This is new behavior — previously the host binary was copied without validation. Go-built Linux binaries use ELFOSABI_NONE which passes the check, so this is transparent in practice, but it's a behavioral change worth noting for awareness. If an exotic Go build configuration produced a different OSABI, this would break an existing working path.

Footer

Outcome: comment-only
This review applies to SHA c105b7fbac28005e6c00bb8a3bd9fa320de55085. Any push to the PR head clears this review and requires a new evaluation.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

When running fullsend on macOS with a Linux sandbox, the pre-agent
security scan fails because the host Mach-O binary cannot execute inside
the Linux container. Add automatic Linux binary resolution with a
4-priority fallback chain:

1. --fullsend-binary <path> (explicit, skips auto-resolution)
2. Download from GitHub Release matching CLI version
3. Cross-compile from source (requires Go toolchain)
4. Download latest release (fallback when cross-compile fails)

Security hardening:
- Path traversal protection in tar extraction (reject ".." and absolute)
- io.LimitReader on compressed stream (200 MB) and extracted binary (500 MB)
- HTTP client with 120s timeout (replaces http.DefaultClient)
- ELF validation: OS/ABI + architecture match against sandbox target
- FULLSEND_SANDBOX_ARCH whitelist (amd64, arm64 only)
- GOMOD=/dev/null edge case handling in cross-compilation
- Oversized binary cleanup on error path

Update local-dev.md with binary resolution docs and FULLSEND_SANDBOX_ARCH
usage.

Signed-off-by: Wayne Sun <gsun@redhat.com>
waynesun09 added 2 commits May 5, 2026 14:34
When resolveLinuxBinary() fails and the harness has fail_mode: closed
(the default), bootstrapSandbox() now returns a hard error instead of
silently skipping the security scan. Only fail_mode: open harnesses
get warning-and-continue behavior.

Downloaded release binaries are now verified against the GoReleaser
checksums.txt before extraction. Checksum mismatch is a hard failure.

Signed-off-by: Wayne Sun <gsun@redhat.com>
Add hex.DecodeString validation and strings.ToLower normalization
to downloadChecksumForAsset. Produces a clear "invalid hex hash"
error instead of a misleading "checksum mismatch" when checksums.txt
contains non-hex characters or uppercase hex.

Signed-off-by: Wayne Sun <gsun@redhat.com>

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.

@waynesun09
waynesun09 requested a review from rh-hemartin May 5, 2026 18:50
@waynesun09
waynesun09 added this pull request to the merge queue May 6, 2026
Merged via the queue into main with commit b120340 May 6, 2026
21 of 22 checks passed
@waynesun09
waynesun09 deleted the fix-cross-platform-security-scan branch May 6, 2026 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants