Skip to content

fix(ipc): validate base64 format in audio:save schema (#250) - #267

Merged
tbrandenburg merged 2 commits into
mainfrom
fix/250-audio-save-base64-validation-integrated
Aug 31, 2026
Merged

fix(ipc): validate base64 format in audio:save schema (#250)#267
tbrandenburg merged 2 commits into
mainfrom
fix/250-audio-save-base64-validation-integrated

Conversation

@tbrandenburg

Copy link
Copy Markdown
Owner

Issues fixed

Closes #250

Summary

audio:save's handler wrapped Buffer.from(base64Audio, "base64") in a try/catch, but Node's lenient base64 decoder never throws, so the catch was dead code. Malformed base64 payloads silently wrote a garbage .webm file to disk instead of surfacing an error.

  • Added a .refine() to saveRecordingSchema.base64Audio in src/shared/schemas.ts validating base64 charset/padding/length (^[A-Za-z0-9+/]*={0,2}$ + length % 4 === 0) before any decode/write happens.
  • Removed the now-fully-dead try/catch around Buffer.from in src/main/ipc.ts's audio:save handler, since validation now happens at the schema boundary.
  • Added a regression test in src/main/ipc.test.ts asserting malformed base64 is rejected with a clear error and no file is written to os.tmpdir().

Validation commands run

  • npx vitest run src/main/ipc.test.ts src/shared/schemas.test.ts → 40/40 passed
  • npx tsc --noEmit -p tsconfig.json && npx tsc --noEmit -p tsconfig.main.json → clean
  • npx vitest run (full suite) → 422 passed, 15 pre-existing failures confined to src/main/model/* (model catalog/registry tests), confirmed to fail identically on main before this change (unrelated, likely network-dependent fixtures)

E2E coverage

No dedicated E2E test added; this is an IPC-boundary validation fix covered by the existing src/main/ipc.test.ts integration-style test (invokes the real IPC handler registry, not mocked), which is the appropriate test layer for this change per repo conventions (renderer only ever sends real base64 from MediaRecorder, so this path isn't reachable through the normal UI flow — see issue for severity rationale).

Risks / follow-ups

  • Risk: low — additive validation only; well-formed base64 (the only input from the real renderer flow) is unaffected; existing tests for valid audio save and mimeType rejection still pass.
  • Follow-up (not fixed here, out of scope): a pre-existing flaky test was observed once during subagent validation ("resolves a bundled extension's provider on the chat path... (issues #211, #212)") that passed on immediate re-run; not reproduced during coordinator's final full-suite run, so no action taken. The 15 src/main/model/* test failures are pre-existing on main and unrelated to this change; no new issue filed since they already fail on main without any modification.

@github-actions

Copy link
Copy Markdown

⚠️ Test duration budget warning

@tbrandenburg the test suite exceeded its duration budget:

  • [SOFT] /home/runner/work/pi-desktop/pi-desktop/src/main/settings/store.test.ts (node) took 2.64s (threshold: 1.0s)

@tbrandenburg
tbrandenburg merged commit ae95608 into main Aug 31, 2026
1 check passed
@tbrandenburg
tbrandenburg deleted the fix/250-audio-save-base64-validation-integrated branch August 31, 2026 04:51
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.

audio:save IPC handler: try/catch around base64 decode is dead code, malformed payloads silently produce garbage files

1 participant