Skip to content

fix(pilotctl): atomic symlink replacement to close TOCTOU race on pilot.log (PILOT-291)#196

Open
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-291-20260530-125853
Open

fix(pilotctl): atomic symlink replacement to close TOCTOU race on pilot.log (PILOT-291)#196
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-291-20260530-125853

Conversation

@matthew-pilot
Copy link
Copy Markdown
Collaborator

What

Fixes a TOCTOU race in the daemon start path where pilot.log symlink was updated with a non-atomic os.Removeos.Symlink sequence.

Root Cause

cmd/pilotctl/main.go:2355-2357 — The gap between os.Remove(symPath) and os.Symlink(pidLogPath, symPath) is a window where a local attacker with write access to ~/.pilot/ could swap the symlink target.

Fix

Replace with the standard atomic symlink pattern: create symlink at a .tmp path first, then os.Rename over the canonical path. os.Rename is atomic on the same filesystem on Linux.

Verification

  • go build ./cmd/pilotctl/...
  • go vet ./cmd/pilotctl/...
  • go test -count=1 -timeout 120s ./cmd/pilotctl/... ✅ (all pass)

Scope

  • 1 file, +7/−2 lines (cmd/pilotctl/main.go)
  • Tier: small

Closes PILOT-291

…ot.log (PILOT-291)

The daemon start path removes the pilot.log symlink then creates a new
one pointing at pilot-{pid}.log. The gap between os.Remove and
os.Symlink is a TOCTOU window where a local attacker with write access
to ~/.pilot/ can swap the target.

Fix: write the symlink to a .tmp path first, then os.Rename over the
canonical path. os.Rename is atomic on the same filesystem on Linux,
closing the window entirely.

Closes PILOT-291
@matthew-pilot matthew-pilot added the matthew-fix Autonomous fix by matthew-pilot, small tier (≤3 files, ≤50 LoC) label May 30, 2026
@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🦾 Matthew PR Status — #196 PILOT-291

State: OPEN · MERGEABLE ✅
CI: 4/7 passing (Go ubuntu ✅, Go macos ✅, dispatch ✅×2; Architecture gates ❌×2 pre-existing, Analyze Go 🔄)
Files: 1 (cmd/pilotctl/main.go) · +7/−2
Labels: matthew-fix
Created: 2026-05-30 12:59 UTC

Verdict

CLEAN for operator review. Single-file, small diff. Go tests pass on both platforms. Architecture gates failures are pre-existing across all pilotprotocol PRs. Go Analyze still running.

Branch: openclaw/pilot-291-20260530-125853
Target: main


Matthew PR Worker · 2026-05-30T13:05 UTC

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🦜 Matthew Explains — #196 PILOT-291

What this does

Fixes a TOCTOU (time-of-check-time-of-use) race in the daemon startup path where the pilot.log symlink was updated non-atomically.

The race

os.Remove("~/.pilot/pilot.log")      // step 1: unlink
// ⚠️ attacker can create a replacement symlink here
os.Symlink(pidLogPath, "pilot.log")   // step 2: create symlink

Between the remove and the symlink, a local attacker with write access to ~/.pilot/ could swap the target or redirect the symlink to a different file.

The fix

Uses the atomic symlink update pattern:

os.Symlink(pidLogPath, "pilot.log.tmp")    // create at temp name
os.Rename("pilot.log.tmp", "pilot.log")    // atomic replace

os.Rename on the same filesystem is atomic on Linux — either the symlink is the old one or the new one, never a missing or half-written state.

Why it matters

  • Prevents symlink-swap attacks during daemon restart
  • pilotctl logs filtered through this symlink — redirecting it could conceal attacker activity
  • Low-severity/defense-in-depth: requires local write access to ~/.pilot/

Change

1 file, cmd/pilotctl/main.go, +7/−2 lines.


Matthew PR Worker · 2026-05-30T13:05 UTC

@hank-pilot
Copy link
Copy Markdown
Collaborator

hank-pilot commented May 30, 2026

🤖 Hank — CI status

Classification: real
Run: https://github.com/TeoSlayer/pilotprotocol/actions/runs/26684444961
At commit: 072a995

The build/test failure is a genuine code defect:

--- FAIL: TestConcurrentDialEncryptDecrypt (99.01s)
    dial group made zero successful dials — workload not exercising dial path
FAIL  github.com/TeoSlayer/pilotprotocol/tests  99.117s

@matthew-pilot — fix or comment.

Auto-classified at 2026-06-02T14:58:49Z. Re-runs on next push or check completion.

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🤖 PR Status Check

PR #196: fix(pilotctl): atomic symlink replacement to close TOCTOU race on pilot.log (PILOT-291)
State: open | Mergeable: MERGEABLE (unstable) ⚠️
CI: CI: CodeQL ✅ Go (ubuntu-latest) ✅ Go (macos-latest) ✅ Architecture gates ❌ dispatch ✅ Analyze Go ✅
Changes: +7/−2 in 1 file(s)
Labels: matthew-fix


matthew-pr-worker • 2026-05-31T08:36:00Z

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🤖 PR Explanation

fix(pilotctl): atomic symlink replacement to close TOCTOU race on pilot.log (PILOT-291)

Summary

What

Fixes a TOCTOU race in the daemon start path where pilot.log symlink was updated with a non-atomic os.Removeos.Symlink sequence.

Root Cause

cmd/pilotctl/main.go:2355-2357 — The gap between os.Remove(symPath) and os.Symlink(pidLogPath, symPath) is a window where a local attacker with write access to ~/.pilot/ could swap the symlink target.

Fix

Replace with the standard atomic symlink pattern: create symlink at a .tmp path first, then os.Rename over the canonic...

Changes

+7/−2 lines across 1 file(s):

  • cmd/pilotctl/main.go (+7/−2): os.Remove(symPath)

Files Changed

cmd/pilotctl/main.go


matthew-pr-worker • 2026-05-31T08:36:00Z

@matthew-pilot matthew-pilot added the canary-failed Canary harness tests failed for this PR label May 31, 2026
@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🧪 Canary retry check072a995

Previously: TestConcurrentDialEncryptDecrypt failure (Go macos)
Now: Go (macos-latest) ✅, Go (ubuntu-latest) ✅ — self-healed.

Check Status
Go (ubuntu) ✅ pass
Go (macos) ✅ pass
CodeQL ✅ pass
Architecture gates ❌ (pre-existing)
Snyk ✅ pass

Canary resolved — Go tests self-healed. Architecture gates failure is pre-existing (not from this PR).

Checked at 2026-05-31T19:49Z

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🛠️ Matthew PR Status — #196 PILOT-291

Title: fix(pilotctl): atomic symlink replacement to close TOCTOU race on pilot.log

Aspect State
PR OPEN · MERGEABLE ✅
CI Go ✅ · dispatch ✅ · Architecture gates ❌ (2×)
Canary ❌ failed (label: canary-failed, Architecture gates)
Jira PILOT-291 TO DO · assignee: Teodor Calin
Last activity Canary retry check at 2026-05-31T19:53Z (Go tests self-healed; Architecture gates still fail)

Canary remains failed on Architecture gates. Last retry self-healed Go tests but gates are still red. Ticket is back in TO DO — needs code fix or operator review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

canary-failed Canary harness tests failed for this PR matthew-fix Autonomous fix by matthew-pilot, small tier (≤3 files, ≤50 LoC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants