Skip to content

Validate sink names and descriptions to prevent config injection - #8

Merged
SimplyJustJessie merged 1 commit into
claude/v2-planning-7a5akzfrom
claude/branch-perf-security-analysis-tjvkdb
Jul 19, 2026
Merged

Validate sink names and descriptions to prevent config injection#8
SimplyJustJessie merged 1 commit into
claude/v2-planning-7a5akzfrom
claude/branch-perf-security-analysis-tjvkdb

Conversation

@SimplyJustJessie

Copy link
Copy Markdown
Owner

Summary

Add comprehensive validation for virtual sink names and descriptions to prevent injection attacks and configuration corruption. This includes stricter validation rules, sanitization of hand-edited configuration files, and new test coverage.

Key Changes

  • Enhanced SinkNameValidator:

    • Replaced simple quote-checking for descriptions with a regex-based pattern that rejects quotes, backslashes, and all Unicode control characters (newlines, tabs, NUL, etc.)
    • Added detailed documentation explaining why each character class is restricted
  • Configuration Sanitization in RuleStore:

    • Added SanitizeVirtualSinks() method that validates all virtual sinks when loading rules.json
    • Drops any sink with an invalid name (prevents module arg injection and reconciler crashes)
    • Replaces invalid descriptions with the sink name (cosmetic fallback, mirrors importer behavior)
    • Logs warnings for dropped/modified sinks
    • Cleaned configuration is persisted on next save
  • Comprehensive Test Coverage:

    • New SinkNameValidatorTests class with theory tests for valid/invalid names and descriptions
    • Tests cover injection vectors: = for module args, quotes for SPA-JSON/shell escaping, backslashes for JSON escaping, and control characters that break the drop-in format
    • New integration test Hand_edited_virtualSinks_are_sanitized_on_load() verifies the sanitization pipeline handles tampered configuration files gracefully

Implementation Details

The validation prevents multiple attack vectors:

  • Name injection: Characters like = could smuggle extra module arguments into pactl argv
  • Quote escaping: Single quotes close the sink_properties value; double quotes close the drop-in's args string
  • Backslash escaping: Collides with SPA-JSON's \\ and \" escaping
  • Control characters: Newlines, tabs, and other control chars break the single-line args = "…" value, causing all declared sinks to fail at boot

The sanitization approach is defensive: even if the UI and importer validate correctly, a hand-edited rules.json file can still carry invalid data. Rather than failing to load, the store sanitizes on load and logs warnings, ensuring the system remains operational.

https://claude.ai/code/session_019fY8PWVacBF6mjfmBmVPnU

Two defense-in-depth gaps in the v2 virtual-sink code:

- IsValidDescription only rejected the two quote styles. A description
  is embedded in the pipewire-pulse drop-in as the double-quoted value
  of a single-line `args = "…"` string, so a newline (easily pasted in
  the create-sink field) breaks that value and — since all sinks share
  one pulse.cmd block — fails every declared sink at boot; a backslash
  collides with SPA-JSON's \" / \\ escaping. Reject backslashes and
  control characters too.

- The load path (RuleStore) only normalized shape, not content, so a
  hand-edited rules.json could feed an unvalidated name straight into
  the pactl argv (module-arg injection) and the generated drop-in, and
  a null name would throw from the reconciler's name-keyed sets and
  disable sink reconcile every pass. Sanitize on load: drop specs with
  an unusable name, fall back to the name for an unusable description.
  The cleaned shape persists on the next save.

Adds SinkNameValidatorTests and a RuleStore sanitization test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019fY8PWVacBF6mjfmBmVPnU
@SimplyJustJessie
SimplyJustJessie merged commit 9353153 into claude/v2-planning-7a5akz Jul 19, 2026
2 checks passed
SimplyJustJessie pushed a commit that referenced this pull request Aug 24, 2026
@SimplyJustJessie
SimplyJustJessie deleted the claude/branch-perf-security-analysis-tjvkdb branch August 24, 2026 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants