fix(security): resolve CodeQL code-scanning alerts - #635
Merged
Conversation
- clear-text-logging (x3): delete leftover apps/landing/paddle-debug.mjs
diagnostic that logged Paddle API key prefix/suffix/length. No references.
- incomplete-multi-character-sanitization (x3): loop the HTML tag/comment
strip until the string stops changing so a re-formed match can't survive
a single pass (html-to-plain-text.ts, app-core markdown.ts, frontmatter.ts).
- insecure-temporary-file (x3): write configs/atomic files via an exclusive
('wx') open with owner-only mode 0o600, matching the already-clean
generate-rpc-bindings writeFileAtomically (file-ops, init, vault-preferences).
- http-to-file-access: restrict remote HTML-import asset downloads to https
and reject loopback/private/link-local hosts before fetching into the vault
(html-importer.ts); destination filename already sanitized.
- bad-code-sanitization: escape U+2028/U+2029 in JSON.stringify'd values used
to build generated RPC bindings (generate-rpc-bindings.ts). Output unchanged.
Two js/file-access-to-http alerts on the test-only e2e claude fixture
dismissed (loopback-guarded, never shipped).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
h4yfans
marked this pull request as ready for review
June 30, 2026 09:23
h4yfans
added a commit
that referenced
this pull request
Jun 30, 2026
Repairs failures introduced by the #635-639 batch merge: - guard app.setAboutPanelOptions (macOS/Linux only) — fixes a Windows startup crash and the main-process unit-test cascade - voice-recorder: init waveform ref at creation, not during render (react-hooks/refs) - video-player: translate the video aria-label (common:media.video) - use-feature-flags, calendar: drop literal extractErrorMessage fallbacks (i18n lint rule + i18n check --max-todo 0) - tests: add useSidebar / useTheme / onMenuCommand mocks for the new menu commands hook; fix the sync vault-id mock (getOrCreateVaultUuid is sync)
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.
Resolves the 13 open CodeQL code-scanning alerts with minimal, correct security fixes. Each file was read before editing.
Fixes (11 alerts)
js/clear-text-loggingx3 —apps/landing/paddle-debug.mjs:21Throwaway diagnostic script logged the Paddle API key (prefix/suffix/length) in cleartext. Grepped the repo: no references anywhere. Deleted the file.
js/incomplete-multi-character-sanitizationx3A single
replace()pass stripping HTML tags/comments can be defeated because removing one match can re-form another from the surrounding text. Fixed by looping the strip until the string stops changing. Behavior for valid input is unchanged; covered by existing tests.apps/desktop/src/main/lib/html-to-plain-text.ts:47(tag strip)packages/app-core/src/markdown.ts:30(HTML comment strip)apps/desktop/src/main/vault/frontmatter.ts:481(HTML comment strip)js/insecure-temporary-filex3CodeQL taint-flows an OS-temp-dir source (test setup uses
fs.mkdtempSync(os.tmpdir())) into these production write sinks. The already-cleangenerate-rpc-bindings.tswriteFileAtomicallyis exempt because it opens withfs.openSync(tempPath, 'wx', 0o600)— exclusive flag + owner-only mode. Applied the samewx+0o600pattern consistently:apps/desktop/src/main/vault/file-ops.ts:36— temp write now{ encoding, mode: 0o600, flag: 'wx' }(random name + rename already present).apps/desktop/src/main/vault/init.ts:98— addedmode: 0o600(keeps create-onlywxsemantics, no clobber).apps/desktop/src/main/vault/vault-preferences.ts:112— converted the direct overwrite into an atomic uniquely-named temp write (wx+0o600) then rename.js/http-to-file-access—apps/desktop/src/main/import/html/html-importer.tsHTML import downloaded remote
<img>refs (await fetch(ref)) and wrote the bytes into the vault, allowing SSRF / writing internal-service content. AddedsafeRemoteAssetUrl(): restricts tohttps, rejects loopback / private / link-local hosts; the http branch now skips unsafe URLs and fetches the validated URL. Destination filename was already sanitized (path.basename+sanitizeFilename), so it stays inside the attachments dir.js/bad-code-sanitization—apps/desktop/scripts/generate-rpc-bindings.ts:78JSON.stringifyused to build generated code leaves U+2028/U+2029 (JS line terminators) unescaped, which could break out of a generated string literal. AddedjsStringLiteral()that escapes them and routed channel/domain/method/property-key literals through it. Generated output is byte-identical (pnpm ipc:generateproduced no diff;pnpm ipc:checkpasses).Dismissed (2 alerts)
js/file-access-to-httpx2 —apps/desktop/tests/e2e/fixtures/claude:56,58Test-only E2E fixture (a fake
claudeCLI), never shipped. It simulates the real Claude CLI reading its--mcp-configand POSTing to the local Memry MCP server — the file-to-http flow is the fixture's entire purpose, and the URL is already constrained to loopback viagetLoopbackUrl(). Dismissed as "won't fix".Verification
pnpm ipc:generate-> no diff;pnpm ipc:checkpasses.pnpm --filter @memry/app-core test(markdown 4/4) andtypecheckpass.PASS (164) FAIL (0)across file-ops, init, vault-preferences, html-importer, html-to-plain-text, attachments.pnpm --filter @memry/desktop typecheck:nodepasses.no-control-regexwarning inhtml-to-plain-text.tsis the pre-existing U+0000 HEADING_MARK regex owned by a sibling lint PR).