Harden agent SVG rendering, secret storage, and provider probes

[snowopsdev]

Summary

• Sanitize agent-supplied svg-options before rendering in the privileged desktop UI while keeping the existing wire shape.
• Restore Electron safeStorage encryption for newly saved secrets, with readable migration support for existing plain: rows and legacy safeStorage/base64 rows.
• Block metadata-service provider probes and require explicit opt-in before probing loopback, private LAN, or link-local custom endpoints.

Security Impact

• Before: model/agent-provided SVG could reach a privileged renderer HTML sink. After: only inert SVG tags and presentation attributes survive sanitization; scripts, event handlers, external references, and unknown tags/attrs are stripped.
• Before: normal secret save flows persisted plain: rows. After: OS-backed safe: rows are written when safeStorage encryption is available, and older readable rows migrate forward on boot.
• Before: renderer-triggered custom provider tests could ask the main process to probe local/private/metadata targets. After: metadata targets fail closed, and private/local targets require an explicit allowPrivateNetwork signal surfaced through the UI.

Compatibility Notes

• svg-options keeps its existing API shape.
• Secret reads continue to support new safe: rows, existing plain: rows, and legacy safeStorage/base64 rows. If OS encryption is unavailable, existing credentials are preserved and plaintext fallback writes log a warning.
• Public HTTPS provider probes are unchanged. Local/private endpoints remain supported after explicit user opt-in, which preserves custom gateways and local provider workflows.

Test Plan

• ✅ corepack pnpm --filter @open-codesign/desktop exec vitest run src/renderer/src/components/AskModal.test.ts src/main/keychain.test.ts src/main/connection-ipc.test.ts src/main/diagnostics-ipc.test.ts src/renderer/src/components/AddCustomProviderModal.test.tsx (161 passed)
• ✅ corepack pnpm --filter @open-codesign/desktop typecheck
• ✅ corepack pnpm lint
• ✅ corepack pnpm audit --prod --audit-level moderate (No known vulnerabilities found)
• ✅ corepack pnpm --filter @open-codesign/desktop build
• ✅ git diff --check
• ⚠️ corepack pnpm --filter @open-codesign/desktop test currently reports 1270/1272 passing with two failures outside this change set: an atimeMs stat equality assertion in src/main/design-workspace.test.ts, and a Puppeteer browser launch failure in src/main/preview-runtime.test.ts.
• ⚠️ corepack pnpm typecheck fails before TypeScript under Turbo with Unable to find package manager binary: cannot find binary path; package-level desktop typecheck passes.

Manual Security Review

• Ran the HTML sink scan: rg -n "dangerouslySetInnerHTML|innerHTML|outerHTML|insertAdjacentHTML|new Function|eval\(" apps packages --glob "!**/vendor/**". The remaining active renderer sink for agent SVG is now sanitized; other matches are tests, static packaged example thumbnails, sandbox/runtime execution paths, DOM capture snippets, or exporter-owned rendering.
• Confirmed normal secret writes now use safe: when safeStorage.isEncryptionAvailable() is true; plain: is fallback/legacy only.
• Confirmed diagnostics redaction treats ciphertext as sensitive regardless of whether the stored row is safe:, legacy safeStorage, or fallback plain:.
• Confirmed private/metadata probe coverage fails closed unless the explicit allow flag is present, and metadata remains blocked even with the flag.

PR Hygiene

• Branch: security/harden-agent-svg-secrets-probes
• Commits:
  • fix: sanitize agent svg options before rendering
  • fix: restore encrypted secret storage migration
  • fix: guard private network provider probes
  • test: cover security hardening paths
• Checked git diff --stat and git diff --check. No generated diagnostic bundles or real secrets are included.

[snowopsdev]

Addressed the CodeQL review comments on AskModal.tsx by removing the regex-based SVG sanitizer fallback entirely. The renderer now uses the DOMParser/XMLSerializer sanitizer path, and non-DOM environments fail closed by returning an empty SVG string instead of attempting regex sanitization.

Validation after the change:

• corepack pnpm --filter @open-codesign/desktop exec vitest run src/renderer/src/components/AskModal.test.ts src/main/keychain.test.ts src/main/connection-ipc.test.ts src/main/diagnostics-ipc.test.ts src/renderer/src/components/AddCustomProviderModal.test.tsx ✅ 161 passed
• corepack pnpm --filter @open-codesign/desktop typecheck ✅
• corepack pnpm lint ✅

[github-actions]

Review mode: initial

Findings

• [Minor] Missing changeset for user-visible changes — The PR restores safeStorage encryption for secrets, adds a new allowPrivateNetwork checkbox in the custom provider modal, and modifies agent SVG rendering behavior. These are user-facing behavioral changes that should be accompanied by a changeset entry (pnpm changeset) to appear in the changelog. Without it, the changelog will miss the update.
  Suggested next action: Run pnpm changeset and commit the generated file. Use a patch bump with a summary of the three hardening changes.

Questions (none)

Summary

• The PR addresses three separate security hardening concerns: sanitizing agent-supplied SVG before rendering in the privileged UI, restoring OS-backed encryption for new secrets (with migration for existing plaintext rows), and blocking private/LAN/metadata provider probes unless explicitly opted in via a new allowPrivateNetwork flag.
• The implementation is well-structured, with clear separation of concerns across AskModal.tsx (SVG sanitization), keychain.ts (secret encryption), and connection-ipc.ts (network target classification).
• All three areas include new Vitest test coverage, and the existing test suites pass (as stated in the PR body).
• The CI workflow is also fixed to ensure the Electron binary is downloaded and installed, addressing a prior silent failure.
• The only actionable issue is the missing changeset. Once a changeset is added, this PR is ready to merge.

Testing

• New unit tests added for sanitizeInlineSvg, classifyNetworkTarget, encryptSecret, and migrateSecrets (with coverage for both encryption-available and encryption-unavailable paths).
• Existing tests pass; the PR body notes two pre-existing test failures unrelated to these changes.
• No E2E tests added, but the scope (security hardening of existing paths) is adequately covered by unit tests.

Open-CoDesign Bot