core/go-io

Fork 0

[audit] Deep audit — stdlib migration gaps, missing tests, error handling #4

New issue

Open

opened 2026-03-22 16:37:32 +00:00 by Virgil · 6 comments

Virgil commented

2026-03-22 16:37:32 +00:00

Member

Prior AX sweep found massive stdlib usage:

2 HIGH: os.Getenv(HOME) in local/client.go
80+ MEDIUM: filepath.* across io.go, node.go, s3.go, local/client.go, datanode/client.go, workspace/service.go, sqlite.go, store/medium.go
10+ LOW: fmt.Errorf, errors.New sentinels

This audit should identify: missing test coverage for all Medium types (local, s3, datanode, sqlite, workspace), error handling gaps, and any additional security concerns.

Prior AX sweep found massive stdlib usage: - 2 HIGH: os.Getenv(HOME) in local/client.go - 80+ MEDIUM: filepath.* across io.go, node.go, s3.go, local/client.go, datanode/client.go, workspace/service.go, sqlite.go, store/medium.go - 10+ LOW: fmt.Errorf, errors.New sentinels This audit should identify: missing test coverage for all Medium types (local, s3, datanode, sqlite, workspace), error handling gaps, and any additional security concerns.

Virgil referenced this issue

2026-03-22 18:12:36 +00:00

[agent/codex] Full audit per issue #4. Read CLAUDE.md. Report ALL findings... #5

Virgil referenced this issue from a commit

2026-03-22 18:12:40 +00:00

Merge pull request '[agent/codex] Full audit per issue #4. Read CLAUDE.md. Report ALL findings...' (#5) from agent/deep-audit-per-issue--4--read-claude-md into dev

Virgil commented

2026-03-22 18:12:56 +00:00

Author

Member

Codex Audit Findings

HIGH (2)

Path traversal in workspace/service.go — name/filename not constrained, ../ escapes sandbox (service.go:42-142)
HOME env guard bypassable in local/client.go Delete/DeleteAll (client.go:268, :280)

MEDIUM (2)

S3 partial delete failure silently ignored (s3/s3.go:199, :233)
Datanode suppresses read/open failures during rename, append, rebuild (datanode/client.go: 6 locations)

## Codex Audit Findings ### HIGH (2) 1. Path traversal in workspace/service.go — name/filename not constrained, ../ escapes sandbox (service.go:42-142) 2. HOME env guard bypassable in local/client.go Delete/DeleteAll (client.go:268, :280) ### MEDIUM (2) 3. S3 partial delete failure silently ignored (s3/s3.go:199, :233) 4. Datanode suppresses read/open failures during rename, append, rebuild (datanode/client.go: 6 locations)

Virgil referenced this issue

2026-03-23 07:26:45 +00:00

[agent/codex] Fix ALL findings from issue #4. Read CLAUDE.md first. Path t... #6

Virgil referenced this issue from a commit

2026-03-23 07:26:50 +00:00

Merge pull request '[agent/codex] Fix ALL findings from issue #4. Read CLAUDE.md first. Path t...' (#6) from agent/deep-audit-per-issue--4--read-claude-md into dev

Virgil commented

2026-03-23 07:27:04 +00:00

Author

Member

Fix Applied

Commit 2acfc3d: fix(io): address audit issue 4 findings

Path traversal in workspace/service.go hardened
HOME env guard fixed in local/client.go
S3 partial delete failures now surfaced
Datanode suppressed failures fixed
590 additions across 11 files with tests

## Fix Applied Commit 2acfc3d: fix(io): address audit issue 4 findings - Path traversal in workspace/service.go hardened - HOME env guard fixed in local/client.go - S3 partial delete failures now surfaced - Datanode suppressed failures fixed - 590 additions across 11 files with tests

Virgil commented

2026-03-23 09:04:17 +00:00

Author

Member

Verification: FAIL (reproduced)

HIGH: Symlink escape still possible — workspacePath does lexical checks only, SwitchWorkspace accepts symlinked workspace entry that escapes root. Review agent reproduced: SwitchWorkspace('evil') with evil -> /tmp/outside.

Needs: resolve symlinks before path validation (filepath.EvalSymlinks or equivalent).

## Verification: FAIL (reproduced) HIGH: Symlink escape still possible — workspacePath does lexical checks only, SwitchWorkspace accepts symlinked workspace entry that escapes root. Review agent reproduced: SwitchWorkspace('evil') with evil -> /tmp/outside. Needs: resolve symlinks before path validation (filepath.EvalSymlinks or equivalent).

Virgil commented

2026-03-23 09:10:29 +00:00

Author

Member

Fix Round 2

Commit def6a8f: Fix workspace symlink escape validation

Resolves symlinks before path boundary checks
49 additions (service.go + test)
Dispatching verification.

## Fix Round 2 Commit def6a8f: Fix workspace symlink escape validation - Resolves symlinks before path boundary checks - 49 additions (service.go + test) Dispatching verification.

Virgil commented

2026-03-23 09:19:04 +00:00

Author

Member

Verification Round 2: FAIL

HIGH: Workspace symlink hardening incomplete — activeFilePath doesn't resolve/revalidate symlinks inside files tree. files dir itself or children can be symlinked to escape. io.Local rooted at / means any resolved path is accessible.

Same TOCTOU pattern as go-rag. Needs architectural decision: fd-based ops or sandboxed io.Local root. Escalating to needs-human.

## Verification Round 2: FAIL HIGH: Workspace symlink hardening incomplete — activeFilePath doesn't resolve/revalidate symlinks inside files tree. files dir itself or children can be symlinked to escape. io.Local rooted at / means any resolved path is accessible. Same TOCTOU pattern as go-rag. Needs architectural decision: fd-based ops or sandboxed io.Local root. Escalating to needs-human.

Virgil commented

2026-03-23 12:14:44 +00:00

Author

Member

Fact-Finding: API Contract Extraction

Agent built a Go AST parser tool to extract the full API surface across all subpackages (local, s3, datanode, sqlite, workspace). The tool parses exported types, functions, methods with coverage detection.

Approach: AST-based extraction with test reference cross-checking. This tool itself could be reusable for contract extraction across all Go repos.

Full source in agent log — consider extracting into a standalone tool.

## Fact-Finding: API Contract Extraction Agent built a Go AST parser tool to extract the full API surface across all subpackages (local, s3, datanode, sqlite, workspace). The tool parses exported types, functions, methods with coverage detection. Approach: AST-based extraction with test reference cross-checking. This tool itself could be reusable for contract extraction across all Go repos. Full source in agent log — consider extracting into a standalone tool.

Virgil referenced this issue from a commit

2026-03-23 14:30:17 +00:00

fix(security): close audit #4 gaps in workspace, local, s3, datanode