Skip to content

fix(crypto): canonicalize identity path via EvalSymlinks (PILOT-295)#12

Open
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-295-20260530-144519
Open

fix(crypto): canonicalize identity path via EvalSymlinks (PILOT-295)#12
matthew-pilot wants to merge 1 commit into
mainfrom
openclaw/pilot-295-20260530-144519

Conversation

@matthew-pilot
Copy link
Copy Markdown
Collaborator

What

SaveIdentity and LoadIdentity now resolve symlinks in the supplied path before operating, preventing writes to unexpected directories when an operator configures IdentityPath as a symlink.

Root cause

crypto/identity.go SaveIdentity (line 84) and LoadIdentity (line 115) used the raw path without symlink canonicalization. If the identity file is a symlink to a directory the operator doesn't fully control, the private key could be written where directory-level permissions (group/ACL) are more permissive than intended.

Fix

Added resolvePath helper that handles three cases:

  1. All components exist → filepath.EvalSymlinks succeeds (fast path)
  2. Final component doesn't exist → resolve parent, re-join base name
  3. Final component is a dangling symlink → follow via os.Readlink

Verification

  • go build ./...
  • go vet ./...
  • go test ./... ✅ (14/14 packages pass)
  • New test: TestSaveLoad_SymlinkResolution — symlink roundtrip

Scope

Files Lines
crypto/identity.go +39
crypto/zz_coverage_test.go +52
Total 2 files, +91

Closes PILOT-295

…lution (PILOT-295)

SaveIdentity and LoadIdentity now resolve any symlinks in the
supplied path before operating. This prevents writes to unexpected
directories when an operator configures IdentityPath as a symlink
to a location they don't fully control (e.g. group-readable dir).

A resolvePath helper handles three cases:
1. All components exist → EvalSymlinks succeeds (fast path).
2. Final component doesn't exist → resolve parent, re-join base.
3. Final component is a dangling symlink → follow via Readlink.

Closes PILOT-295
@matthew-pilot matthew-pilot added the matthew-fix-larger Medium-scope fix (≤10 files, ≤200 LoC) — operator review with diff stat label May 30, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 30, 2026

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crypto/identity.go 94.11% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

📊 PR Status — #12 PILOT-295

Field Value
State OPEN
Mergeable ✅ MERGEABLE
Draft No
Branch openclaw/pilot-295-20260530-144519main
Files 2 files, +91/−0
Labels matthew-fix-larger
Author @matthew-pilot

CI Checks (2/2 passing)

Check Result
codecov/patch ✅ pass
test ✅ pass

Files Changed

  • crypto/identity.go (+39/-0)
  • crypto/zz_coverage_test.go (+52/-0)

🤖 Auto-generated by matthew-pr-worker | 2026-05-30T15:12:00Z

@matthew-pilot
Copy link
Copy Markdown
Collaborator Author

🔍 PR Explanation — #12 PILOT-295

What this does

fix(crypto): canonicalize identity path via EvalSymlinks (PILOT-295)

Scope

  • Files: 2 files
  • Delta: +91/−0 lines
  • Labels: matthew-fix-larger
  • Mergeable: MERGEABLE

Tickets

Files

  • crypto/identity.go (+39/-0)
  • crypto/zz_coverage_test.go (+52/-0)

Review Notes

  • This is an automated code-maintenance PR from matthew-pilot
  • Operator review required before merge
  • Check CI status and canary results above

🤖 Auto-generated by matthew-pr-worker | 2026-05-30T15:12:00Z

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

Labels

matthew-fix-larger Medium-scope fix (≤10 files, ≤200 LoC) — operator review with diff stat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant