Security findings in storage layer, self-update, and config handling (PatchPilots audit)

[alavesa]

Security findings from PatchPilots audit

I ran PatchPilots (an AI code review tool) against the git-lrc codebase — 20 files across storage, config, and network layers. Risk score: HIGH.

High severity

1. Self-update binary path not validated (storage/selfupdate_schema.go)

PendingUpdateState serializes StagedBinaryPath to a JSON state file on disk. If the state file is in a writable location, an attacker can replace StagedBinaryPath with an arbitrary path pointing to a malicious binary. When the update is applied, the attacker's binary gets executed.

2. Path traversal in attestation file reading (storage/attestation_read_io.go)

ReadAttestationFile passes a file path directly to os.ReadFile without sanitization or boundary validation. If the path is derived from user input, an attacker can read arbitrary files on the system.

Medium severity

3. JWT and refresh tokens stored in plain text (ui/config.go)

LoadRuntimeConfig reads JWT access tokens and refresh tokens from ~/.lrc.toml into plain-text string fields. The config file is not created with explicitly restricted permissions.

4. API key serialized in plain text (ui/types.go)

The ConnectorRemote struct exposes an APIKey field tagged with json:"api_key". Whenever this struct is marshaled to JSON (e.g., logging, debug output, --save-json flag), the API key is included in plain text.

5. SQLite DSN injection via unsanitized dbPath (storage/files.go, storage/attestation_review_db_io.go)

OpenSQLite and OpenAttestationReviewDB interpolate dbPath directly into the DSN string using fmt.Sprintf without sanitization. If dbPath contains a ? character, an attacker can inject arbitrary SQLite pragmas or parameters.

6. Path traversal via unsanitized metaFilename (storage/hook_io.go)

ReadHookMetaFile and RemoveHookMetaFile construct paths using filepath.Join(hooksPath, metaFilename) without validating the result stays within hooksPath. A malicious metaFilename with .. components can escape the intended directory.

7. Integrity hash is optional — allows bypass (storage/selfupdate_schema.go)

The IntegrityHash field uses omitempty, meaning it can be absent from the JSON. If downstream update logic treats an absent hash as "no check needed", an attacker can bypass integrity verification of the staged binary.

8. No path validation in file read/delete wrappers (storage/file_read_io.go, storage/file_delete_io.go)

Functions like ReadConfigFile, RemoveTempHTMLFile, RemoveHookScriptFile, and others accept arbitrary path strings and pass them directly to os.ReadFile or os.Remove without boundary validation.

Low severity

• API key exposed via CLI flag visible in process list (main.go)
• HTTP server may bind to all interfaces (main.go)
• SQL query text included in error messages (storage/files.go)
• Config file path and internal errors exposed in ConfigErr field (ui/config.go)
• TOCTOU race condition in RemoveDirIfEmptyIfExists (storage/uninstall_io.go)

---

Tool: PatchPilots v0.7.3 — 8 AI agents for automated code review
Audit scope: 20 files, reviewer + security agents
Risk score: HIGH
Cost: ~$1.36 in Claude API tokens