fix: resolveInsideRoot uses filepath.EvalSymlinks to close CWE-59
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 12s
Harness Replays / detect-changes (pull_request) Successful in 12s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 11s
CI / Detect changes (pull_request) Successful in 36s
E2E API Smoke Test / detect-changes (pull_request) Successful in 35s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 34s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 34s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 17s
Harness Replays / Harness Replays (pull_request) Successful in 7s
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: 7
qa-review / approved (pull_request) Failing after 20s
sop-checklist-gate / gate (pull_request) Successful in 18s
security-review / approved (pull_request) Failing after 21s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 30s
sop-tier-check / tier-check (pull_request) Successful in 24s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 7s
CI / Canvas (Next.js) (pull_request) Successful in 9s
gate-check-v3 / gate-check (pull_request) Failing after 32s
CI / Python Lint & Test (pull_request) Successful in 9s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 11s
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Failing after 1m25s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m28s
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 1m47s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 8s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Failing after 4m44s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 5m37s
CI / Platform (Go) (pull_request) Successful in 11m58s
CI / all-required (pull_request) Successful in 3s
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 12s
Harness Replays / detect-changes (pull_request) Successful in 12s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 11s
CI / Detect changes (pull_request) Successful in 36s
E2E API Smoke Test / detect-changes (pull_request) Successful in 35s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 34s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 34s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 17s
Harness Replays / Harness Replays (pull_request) Successful in 7s
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: 7
qa-review / approved (pull_request) Failing after 20s
sop-checklist-gate / gate (pull_request) Successful in 18s
security-review / approved (pull_request) Failing after 21s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 30s
sop-tier-check / tier-check (pull_request) Successful in 24s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 7s
CI / Canvas (Next.js) (pull_request) Successful in 9s
gate-check-v3 / gate-check (pull_request) Failing after 32s
CI / Python Lint & Test (pull_request) Successful in 9s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 11s
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Failing after 1m25s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m28s
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 1m47s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 8s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Failing after 4m44s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 5m37s
CI / Platform (Go) (pull_request) Successful in 11m58s
CI / all-required (pull_request) Successful in 3s
The pre-existing resolveInsideRoot (org_helpers.go) used only filepath.Abs, which does NOT resolve symlinks on Unix. A symlink planted inside the workspace that points outside (e.g. workspaces/dev/leaked → /etc) passed the lexical prefix check because /tmp/.../workspaces/dev/leaked lexically starts with /tmp/.../. Add filepath.EvalSymlinks after the lexical check: 1. Lexical check catches obvious .. escapes. 2. EvalSymlinks resolves symlinks; fails on broken symlinks. 3. Re-check the resolved path against absRoot — catches planted outbound symlinks (CWE-59). Broken symlinks are rejected because EvalSymlinks returns an error, which propagates as "symlink resolve failed". This matches the regression test added in this PR. Without this fix, TestResolveInsideRoot_RejectsSymlinkTraversal (the CWE-59 regression test added alongside) FAILS on any Unix system where /tmp is a real directory (symlink test returns nil instead of error), causing CI/Platform (Go) to fail and blocking the continue-on-error unmask needed for Phase 4. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
f23d49d540
commit
851fcbfa32
@ -319,15 +319,16 @@ func mergePlugins(defaultPlugins, wsPlugins []string) []string {
|
||||
return out
|
||||
}
|
||||
|
||||
// resolveInsideRoot joins `userPath` onto `root` and ensures the lexically
|
||||
// cleaned result stays inside root. Rejects absolute paths outright and
|
||||
// anything containing ".." that would escape the root.
|
||||
// resolveInsideRoot joins `userPath` onto `root` and ensures the resolved
|
||||
// result stays inside root. Rejects absolute paths outright.
|
||||
//
|
||||
// Both arguments are resolved to absolute paths via filepath.Abs before the
|
||||
// prefix check so a root passed as a relative path still works correctly.
|
||||
// Follows Go's standard pattern for SSRF-class path sanitization; using
|
||||
// strings.HasPrefix on an absolute-path pair plus the separator guard rejects
|
||||
// sibling directories that share a prefix (e.g. "/foo" vs "/foobar").
|
||||
// After the lexical check, filepath.EvalSymlinks is called on the joined path
|
||||
// to resolve any symlinks; the final resolved path is then checked against
|
||||
// root. This closes CWE-59 (symlink-based path traversal): a symlink planted
|
||||
// inside the workspace that points outside the root is rejected, and broken
|
||||
// symlinks are also rejected since they cannot be valid org files.
|
||||
func resolveInsideRoot(root, userPath string) (string, error) {
|
||||
if userPath == "" {
|
||||
return "", fmt.Errorf("path is empty")
|
||||
@ -344,9 +345,24 @@ func resolveInsideRoot(root, userPath string) (string, error) {
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("joined abs: %w", err)
|
||||
}
|
||||
// Allow exact-root match (rare but valid) and any descendant.
|
||||
// Lexical check: reject obvious escapes before the more expensive symlink walk.
|
||||
if absJoined != absRoot && !strings.HasPrefix(absJoined, absRoot+string(filepath.Separator)) {
|
||||
return "", fmt.Errorf("path escapes root")
|
||||
}
|
||||
return absJoined, nil
|
||||
// Resolve symlinks. EvalSymlinks also fails on broken symlinks — those
|
||||
// cannot be valid org files, so they are rejected.
|
||||
resolved, err := filepath.EvalSymlinks(absJoined)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("symlink resolve failed: %w", err)
|
||||
}
|
||||
// Re-check after symlink resolution: a symlink inside the root that points
|
||||
// outside must be caught here (e.g. workspaces/dev/leaked → /etc).
|
||||
resolvedAbs, err := filepath.Abs(resolved)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("resolved abs: %w", err)
|
||||
}
|
||||
if resolvedAbs != absRoot && !strings.HasPrefix(resolvedAbs, absRoot+string(filepath.Separator)) {
|
||||
return "", fmt.Errorf("resolved path escapes root")
|
||||
}
|
||||
return resolvedAbs, nil
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user