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

New issue

Open

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

Virgil commented

2026-03-22 16:41:03 +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 commented

2026-03-22 19:31:20 +00:00

Author

Member

Codex Audit Findings

MEDIUM — AX Compliance (extensive)

stdlib usage throughout production code:

backend.go: strings.HasPrefix (6 hits)
discover.go: filepath.Glob, filepath.Join
model.go: strings.Builder, fmt.Sprintf
server.go: os.Getenv, strings.HasPrefix, fmt.Sprintf
vram.go: filepath.Glob/Dir/Join, strings.TrimSpace
internal/gguf/gguf.go: fmt.Sprintf, strings.HasSuffix (17 hits)
internal/llamacpp/health.go: strings.TrimRight, fmt.Sprintf
internal/llamacpp/client.go: fmt.Sprintf, strings.TrimSpace/HasPrefix/TrimPrefix

Same violations in test code (server_test.go, discover_test.go)

## Codex Audit Findings ### MEDIUM — AX Compliance (extensive) stdlib usage throughout production code: - backend.go: strings.HasPrefix (6 hits) - discover.go: filepath.Glob, filepath.Join - model.go: strings.Builder, fmt.Sprintf - server.go: os.Getenv, strings.HasPrefix, fmt.Sprintf - vram.go: filepath.Glob/Dir/Join, strings.TrimSpace - internal/gguf/gguf.go: fmt.Sprintf, strings.HasSuffix (17 hits) - internal/llamacpp/health.go: strings.TrimRight, fmt.Sprintf - internal/llamacpp/client.go: fmt.Sprintf, strings.TrimSpace/HasPrefix/TrimPrefix Same violations in test code (server_test.go, discover_test.go)

Virgil commented

2026-03-23 08:03:33 +00:00

Author

Member

Fix Applied

Commit b788412: fix: resolve issue 2 audit findings

Full AX compliance: os.Getenv, filepath., fmt.Sprintf, strings. all replaced with core.* across entire codebase
New internal/core/core.go helper package (81 lines)
model.go substantially refactored (+376/-375 lines)
llamacpp/client.go hardened (+307 lines)
model_test.go added (202 lines), server_test.go expanded (197 lines)
SPDX headers added
1,311 additions across 23 files

## Fix Applied Commit b788412: fix: resolve issue 2 audit findings - Full AX compliance: os.Getenv, filepath.*, fmt.Sprintf, strings.* all replaced with core.* across entire codebase - New internal/core/core.go helper package (81 lines) - model.go substantially refactored (+376/-375 lines) - llamacpp/client.go hardened (+307 lines) - model_test.go added (202 lines), server_test.go expanded (197 lines) - SPDX headers added - 1,311 additions across 23 files

Virgil commented

2026-03-23 09:16:31 +00:00

Author

Member

Verification: FAIL

HIGH: BatchGenerate violates TextModel contract — collapses each prompt's completion into single Token instead of per-token output. Interface expects per-token output (go-inference TextModel contract).

Needs: return individual tokens per prompt, not aggregated single token.

## Verification: FAIL HIGH: BatchGenerate violates TextModel contract — collapses each prompt's completion into single Token instead of per-token output. Interface expects per-token output (go-inference TextModel contract). Needs: return individual tokens per prompt, not aggregated single token.

Virgil commented

2026-03-23 09:25:54 +00:00

Author

Member

Fix Round 2

Commit 3b18b33: fix: preserve batch token boundaries

BatchGenerate now returns per-token output matching TextModel contract
38-line model_test.go updated to verify token boundaries
Dispatching verification.

## Fix Round 2 Commit 3b18b33: fix: preserve batch token boundaries - BatchGenerate now returns per-token output matching TextModel contract - 38-line model_test.go updated to verify token boundaries Dispatching verification.

Virgil commented

2026-03-23 09:34:46 +00:00

Author

Member

Verification Round 2: FAIL (medium)

MEDIUM: Classify with WithLogits() breaks when sampling options present — temperature/top_k/top_p affect logprob output from llama.cpp /completion. top_logprobs may have fewer entries than vocab size.

This is llama.cpp API semantics — needs human understanding of the sampling→logprobs interaction. Escalating to needs-human.

## Verification Round 2: FAIL (medium) MEDIUM: Classify with WithLogits() breaks when sampling options present — temperature/top_k/top_p affect logprob output from llama.cpp /completion. top_logprobs may have fewer entries than vocab size. This is llama.cpp API semantics — needs human understanding of the sampling→logprobs interaction. Escalating to needs-human.