[audit] Security, AX compliance, missing tests, error handling #8

New issue

Open

opened 2026-03-22 16:41:01 +00:00 by Virgil · 5 comments

Virgil commented

2026-03-22 16:41:01 +00:00

Member

Full audit:

Security: path traversal, injection, panics on untrusted input, race conditions
AX compliance: os.Getenv → core.Env, filepath.* → core.Path*, fmt.Sprintf → core.Sprintf, strings.* → core.*, errors.New/fmt.Errorf → core.E
Missing tests: exported functions without test coverage
Error handling: silently dropped errors, bare panics, missing nil checks
UK English: American spellings in comments/docs
Missing usage-example comments on exported identifiers
Missing SPDX licence headers

Report all findings with severity and file:line. Do NOT fix.

Full audit: 1. Security: path traversal, injection, panics on untrusted input, race conditions 2. AX compliance: os.Getenv → core.Env, filepath.* → core.Path*, fmt.Sprintf → core.Sprintf, strings.* → core.*, errors.New/fmt.Errorf → core.E 3. Missing tests: exported functions without test coverage 4. Error handling: silently dropped errors, bare panics, missing nil checks 5. UK English: American spellings in comments/docs 6. Missing usage-example comments on exported identifiers 7. Missing SPDX licence headers Report all findings with severity and file:line. Do NOT fix.

Virgil referenced this issue

2026-03-22 18:17:10 +00:00

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

Virgil commented

2026-03-22 18:48:03 +00:00

Author

Member

Codex Audit Findings

HIGH

Path injection — user-controlled URL segments interpolated directly into API paths without escaping. Affects: contents.go (5 locations), wiki.go (3 locations), actions.go, branches.go, commits.go, issues.go, labels.go, misc.go, notifications.go, orgs.go, packages.go, pulls.go, releases.go, repos.go — nearly every service file. / ? #` can alter target endpoint.

## Codex Audit Findings ### HIGH 1. Path injection — user-controlled URL segments interpolated directly into API paths without escaping. Affects: contents.go (5 locations), wiki.go (3 locations), actions.go, branches.go, commits.go, issues.go, labels.go, misc.go, notifications.go, orgs.go, packages.go, pulls.go, releases.go, repos.go — nearly every service file. /` ?` #` can alter target endpoint.

Virgil commented

2026-03-23 06:48:23 +00:00

Author

Member

Fix Applied

Commit 9cdab89: Fix service path segment escaping

All 19 service files updated to use url.PathEscape on path segments
New path_escape.go helper for consistent escaping
257-line path_escape_test.go covering injection vectors
Files: actions, branches, commits, contents, issues, labels, misc, notifications, orgs, packages, pulls, releases, repos, teams, users, webhooks, wiki

## Fix Applied Commit 9cdab89: Fix service path segment escaping - All 19 service files updated to use url.PathEscape on path segments - New path_escape.go helper for consistent escaping - 257-line path_escape_test.go covering injection vectors - Files: actions, branches, commits, contents, issues, labels, misc, notifications, orgs, packages, pulls, releases, repos, teams, users, webhooks, wiki

Virgil commented

2026-03-23 08:53:42 +00:00

Author

Member

Verification: FAIL

go test passes but misleadingly — one added test asserts wrong combined-status path, escaping tests don't exercise router behaviour. Needs test fixes.

## Verification: FAIL go test passes but misleadingly — one added test asserts wrong combined-status path, escaping tests don't exercise router behaviour. Needs test fixes.

Virgil commented

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

Author

Member

Fix Round 2

Commit 7e5717d: Fix combined status path tests

Fixed test assertion for combined status
Added 109-line path_escape_test.go with router integration tests
Dispatching verification.

## Fix Round 2 Commit 7e5717d: Fix combined status path tests - Fixed test assertion for combined status - Added 109-line path_escape_test.go with router integration tests Dispatching verification.

Virgil commented

2026-03-23 09:08:32 +00:00

Author

Member