Phase 3: opt-in script post-processor (fail-open) - #54
Merged
Conversation
Final Phase 3 item — completes the hackability phase. Lets power users pipe the
final transcript through any executable just before insertion.
- ScriptOutcome (OpenWhispCore, pure): given (stdout, exitCode, timedOut,
launchFailed), decides whether to use the script output or keep the original.
Fail-open by contract — launch failure, timeout, non-zero exit, nil exit, and
empty/whitespace output all keep the original transcript. Strips exactly one
trailing newline (the echo convention), preserving other whitespace.
- ScriptPathValidator (pure, injectable fs checks): empty / not-found /
not-executable / ok, for inline Settings validation.
- ScriptRunner (app-side glue): spawns the script via Process, writes stdin,
reads stdout off a background queue (no pipe-buffer deadlock), enforces a ~2s
timeout with SIGTERM->SIGKILL, and resolves through ScriptOutcome.
- AppState: scriptPostProcessorEnabled / scriptPostProcessorPath settings; runs
the script at the single insertion choke point (insertCompletedText), after the
secure-field guard so it never sees password-field content. Off by default.
- SettingsView: "Script Post-processor" section (Advanced) — toggle, file picker,
live path validation, and a trust warning.
- Tests: +15 (ScriptOutcome all fail-open branches + newline handling;
ScriptPathValidator). The CRLF newline test caught a real grapheme bug
("\r\n" is ONE Character, so dropLast(2) ate the preceding char) — fixed to a
grapheme-correct single dropLast. 156 -> 171. ScriptRunner additionally verified
end-to-end against real scripts (uppercase / empty / exit-7 / missing / 1s
timeout that actually fires at ~1.01s).
- Phase 3 marked complete; README + ROADMAP updated.
swift test 171/171; ./build.sh clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Security review of the script post-processor found two should-fix issues sharing one root cause: stdoutData was read even when the background reader hadn't finished (a data race on the timeout path), and a killed script that left a grandchild holding the stdout pipe open would orphan the reader thread + FDs. - Only read stdoutData AFTER readGroup.wait() returns success (establishes the happens-before edge). On the timeout path, force-close the read end to make the blocked readDataToEndOfFile return EOF, then wait for the now-unblocked reader. - Put the child in its own process group (setpgid) and SIGKILL the whole group (kill(-pid)) plus the child directly, so daemonized grandchildren are reaped. - Redirect stderr to /dev/null (was an undrained Pipe) so a script flooding stderr can't block on a full buffer and force a needless timeout. Verified end-to-end against real scripts: uppercase transform, empty/exit-7/ missing (fail-open), 1s timeout fires at ~1.01s, a grandchild-holds-pipe script now returns in ~0.78s (was a 5s hang), and a 100k-line-stderr script completes in ~0.74s. swift test 171/171; build clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Hardening pass (after a security review of the spawn path):
Verified end-to-end: the grandchild-holds-pipe case now returns in ~0.78s (was a 5s hang), 100k-line-stderr completes in ~0.74s, timeout fires at ~1.01s. The core safety model (opt-in, fail-open, secure-field-first, no shell) was confirmed sound by the review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Final Phase 3 item — completes the hackability phase. Pipe the final transcript through any executable just before insertion (your text on stdin, result on stdout).
Safety model (per the agreed design)
Changes
ScriptOutcome(OpenWhispCore, pure): decides use-output vs keep-original from (stdout, exitCode, timedOut, launchFailed). Strips exactly one trailing newline, preserving other whitespace.ScriptPathValidator(pure, injectable fs checks) for inline Settings validation.ScriptRunner(app-side):Process+ stdin write + background stdout read (no pipe-buffer deadlock) + timeout, resolving throughScriptOutcome.scriptPostProcessorEnabled/scriptPostProcessorPath; runs at the single insertion choke point."\r\n"is oneCharacter, sodropLast(2)ate the preceding char; fixed to a grapheme-correct singledropLast. 156 → 171.ScriptRunneralso verified end-to-end against real scripts (uppercase / empty / exit-7 / missing / a 1s timeout that fires at ~1.01s, not 5s).swift test171/171 ·./build.shclean.🤖 Generated with Claude Code