cowork: fix preview temp-file leak, crypto webview nonces, detect-format normalization - #14
Open
github-actions[bot] wants to merge 2 commits into
Open
cowork: fix preview temp-file leak, crypto webview nonces, detect-format normalization#14github-actions[bot] wants to merge 2 commits into
github-actions[bot] wants to merge 2 commits into
Conversation
…normalization - schemaEditorProvider: temp scratch files now created via mkdtemp under os.tmpdir() and removed in a finally block; previously every preview render/save leaked a schemaforge_preview_*.tmp into <installDir>/.temp - webview nonces: crypto.randomBytes instead of predictable Math.random; getNonce/escapeHtml deduplicated into shared src/webview.ts so the two preview surfaces cannot drift apart again - detection robustness: normalizeFormat applied before convert --from in quickConvert/diff/schemaEditorProvider; empty detection now asks the user (convert) or aborts with a clear message (quickConvert/diff) instead of passing an opaque empty --from to the CLI - provider reuses canonical SCHEMA_FORMATS list (was an inline copy) - removed unused execSchemaForgeSync which silently swallowed CLI errors - tests: functional webview util tests + source-level regression guards for temp lifecycle and nonce randomness; tsc strict clean, eslint clean, 15/15 node --test pass
When schemaforge.cliPath points at a .cmd/.bat wrapper on Windows, execFile runs with shell:true and Node flattens argv into one 'cmd.exe /c' line without quoting. A file path containing cmd metacharacters (e.g. foo&calc.bat) could execute arbitrary commands. - quote every argument in shell mode; reject metacharacter-bearing arguments with a clear error before any process is spawned - export quoteShellArg for testability; add source-level regression tests locking the guard in tsc strict clean, eslint clean, 17/17 tests pass.
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 freeto 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.
Summary
One coherent correctness + hardening round on the preview/convert pipeline (commit a02c8a1).
Fixed (correctness bugs)
schemaforge.previewEditorcustom editor wrote aschemaforge_preview_<ts>.tmpinto<installDir>/.tempon every render and save, and never deleted any of them. Scratch files now go throughfs.promises.mkdtempunderos.tmpdir()and the directory is removed in afinallyblock.--fromflag, failing opaquely. Convert now asks the user to pick the source format; QuickConvert and Diff abort with a clear message instead.Hardened (security)
Math.random(); both preview surfaces now usecrypto.randomBytes(16). Nonce generation and HTML escaping moved into one shared module (src/webview.ts) so the two implementations cannot drift again.Changed (silent-failure / consistency)
normalizeFormatis applied to detected formats before they reachconvert --fromin QuickConvert, Diff and the.schemaforgeeditor (mixed-case/padded labels previously passed through verbatim in 3 of 5 call sites).SCHEMA_FORMATSlist instead of its own inline copy (the exact drift classsrc/formats.tsdocuments).execSchemaForgeSync, which silently swallowed CLI errors.Tests
mkdtemp/os.tmpdir()withfinally { rmSync }cleanup; noMath.random-based nonces; shared helper import required.tsc -p .strict clean, eslint clean, 15/15 node --test pass.