Skip to content

security: three HIGH path-injection alerts in skills_catalog.go (+ log-injection sweep) #1771

Description

@asheshgoplani

CodeQL Advanced Security reports three HIGH-severity go/path-injection alerts in internal/session/skills_catalog.go (lines ~1080, ~1115, ~1161): 'This path depends on a user-provided value.' These are pre-existing (surfaced while triaging an unrelated PR's scan), in real code rather than tests.

Ask: audit each site — a skill name/source from user config or a fetched catalog reaching a filesystem path needs containment (validate against a strict charset, reject separators and .., and confirm the resolved path stays inside the intended skills root via filepath.Clean plus a prefix check on the resolved absolute path). If a site is genuinely safe by construction, document why inline and dismiss the alert with that reason rather than leaving it open.

Also of note: several medium go/log-injection alerts exist (internal/web/handlers_ws.go:202/211, internal/tmux/tmux.go:1835, internal/session/opencode_mcp.go:228) — worth a separate sweep to sanitize newlines/control characters out of logged user-provided values.

Note for triage: the path-expression alert reported on PR #1769 is a FALSE POSITIVE — that PR's flagged paths are t.TempDir() values inside new test files.

Metadata

Metadata

Assignees

No one assigned

    Labels

    securitySecurity-related issues and CVEs

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions