Skip to content

fix(cli): publish cached synthesis script exclusively - #3722

Merged
jrusso1020 merged 1 commit into
mainfrom
fix/security-tts-script-create
Sep 6, 2026
Merged

fix(cli): publish cached synthesis script exclusively#3722
jrusso1020 merged 1 commit into
mainfrom
fix/security-tts-script-create

Conversation

@jrusso1020

Copy link
Copy Markdown
Collaborator

The TTS cache checks for synth-v3.py before writing it. A concurrent script can be truncated, and a dangling symlink can cause its missing target to be created.

Publish the cached script with the existing writeNewFileSync helper. This reuses private staging and exclusive hard-link publication while preserving the existing-cache shortcut, script body, version cleanup, and synthesis arguments. No workflow changes.

Addresses CodeQL #269.

Validation: all 35 TTS and shared-publication regression tests pass. Three new public-synthesis cache cases cover creation/reuse/old-version cleanup, a concurrent winner, and dangling-symlink target non-creation. The two security witnesses fail on main. These tests use real temporary files and stub Python execution/model acquisition; they do not run Kokoro or download models. CLI typecheck, parser/core/lint builds, lint/format and complexity audit pass. Windows CI, CodeQL and Magi approval remain merge gates.

As with the helper's other callers, the cache filesystem must support hard links; publication errors propagate instead of overwriting a destination entry.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at exact head 835a7dad6e555b6e7e34367c88c615c633e03fd6. No blockers.

The cache protocol at packages/cli/src/tts/synthesize.ts:95-122 is compatible and correctly ordered:

  • an existing synth-v3.py still returns immediately, without directory creation or old-version cleanup;
  • a cache miss creates the parent, then delegates the only script publication to the Windows-verified writeNewFileSync boundary;
  • successful publication or a link-time EEXIST winner proceeds to best-effort removal of only older synth(-vN)?.py entries;
  • unsupported hard links/write/publication errors propagate before cleanup or Python execution;
  • helper staging cleanup remains guaranteed on every post-mkdtemp exit.

The inline Python literal is byte-unchanged, and ensureSynthScript still returns the same synth-v3.py path passed to the unchanged model/voices/text/voice/speed/output/lang argv at synthesize.ts:189-205. A repository scan finds no sibling synth-script writer.

The public cache tests at synthesize.cache.test.ts:55-105 use real filesystem entries and pin creation bytes, cache reuse, selective old-version cleanup, concurrent winner preservation, dangling-link target non-creation, no staging residue, and the final Python argv while model acquisition/execution remain stubbed.

Independent exact-head verification: 35/35 TTS/shared-publication tests and changed-file formatting passed. The unchanged Windows studio-engine-cli lane runs the full CLI suite and is still in progress; it remains the landing gate for cross-platform hard-link behavior.

CI has no completed failure; JavaScript CodeQL, Windows, build/typecheck, and remaining jobs are still running.

Verdict: APPROVE
Reasoning: The sole synthesis-script creation path now reuses the proven atomic publication boundary without changing cache hits, script bytes, version cleanup, or execution arguments.

— Magi

@jrusso1020
jrusso1020 merged commit cfdaccb into main Sep 6, 2026
49 checks passed
@jrusso1020
jrusso1020 deleted the fix/security-tts-script-create branch September 6, 2026 00:05
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