PCH-02-001 — UPDATE_ENTRY patch path routes canary/tags/passkey through existing sanitizers

Severity: Low (the only non-Informational finding in PCH-02) Target file: src/background/vault.ts — sanitizeEntryPatch, lines 2375–2388 Report: peach-code-review.md, Finding PCH-02-001

Problem

sanitizeEntryPatch copies patch.canary (2375–2377), patch.passkey (2379–2384), and patch.tags (2386–2388) through verbatim, while every other vault-entry funnel rebuilds those shapes field-by-field:

• add / batch-add → sanitizeEntryDraft (vault.ts:2256–2315) runs sanitizeCanary + sanitizeTags (PCH-01-016) and throws on invalid passkey
• restore / import / sync → sanitizeRestoredEntry (vault.ts:1603–1650) runs sanitizePasskey + sanitizeCanary + sanitizeTags (MV-6)

UPDATE_ENTRY is legitimately sent from content-script context (inline editor, canary-ping bump at content/index.ts:336–348), so it cannot be added to DESTRUCTIVE_MESSAGE_TYPES — the fix is to make the patch path use the same sanitizers, not to gate the sender.

Fix — exact edit

In sanitizeEntryPatch (vault.ts), replace the three verbatim branches:

// BEFORE (vault.ts:2375-2388)
  if ("canary" in patch) {
    nextPatch.canary = patch.canary;
  }

  if ("passkey" in patch) {
    // Passkey sub-object replaces all the legacy flat passkey fields. A
    // patch with `passkey: undefined` removes the credential from the entry
    // (used by "remove passkey" without deleting the whole entry).
    nextPatch.passkey = patch.passkey;
  }

  if ("tags" in patch) {
    nextPatch.tags = patch.tags;
  }

// AFTER
  if ("canary" in patch) {
    // PCH-02-001: route the patch canary through the same MV-6 whitelist the
    // add/restore/sync funnels use. An invalid shape (non-https webhook,
    // missing webhookUrl) drops the canary — mirroring sanitizeRestoredEntry —
    // rather than rejecting the whole patch. `canary: undefined` still clears.
    nextPatch.canary = patch.canary ? sanitizeCanary(patch.canary) : undefined;
  }

  if ("passkey" in patch) {
    // Passkey sub-object replaces all the legacy flat passkey fields. A
    // patch with `passkey: undefined` removes the credential from the entry
    // (used by "remove passkey" without deleting the whole entry).
    // PCH-02-001: a DEFINED passkey must pass the same field-by-field rebuild
    // the restore path applies. Refuse the patch on an invalid shape rather
    // than silently replacing the stored credential with a malformed one —
    // mirrors sanitizeEntryDraft, which throws "invalid passkey credential".
    if (patch.passkey === undefined || patch.passkey === null) {
      nextPatch.passkey = undefined;
    } else {
      const sanitized = sanitizePasskey(patch.passkey);
      if (!sanitized) throw new Error("invalid passkey credential");
      nextPatch.passkey = sanitized;
    }
  }

  if ("tags" in patch) {
    // PCH-02-001: bound tags exactly like both import funnels (PCH-01-016) —
    // strings only, ≤256 chars each, ≤255 tags. Non-array collapses to
    // undefined, so `tags: []` (and junk shapes) clear the field.
    nextPatch.tags = sanitizeTags(patch.tags);
  }

No import changes needed — sanitizeCanary, sanitizePasskey, and sanitizeTags are module-local functions in the same file (vault.ts:1657, 1705, 1730).

Compatibility audit of every existing UPDATE_ENTRY producer

Verified against source before choosing these semantics:

Producer	Patch shape	Survives?
Content canary-ping bump (content/index.ts:336–348)	full existing canary + lastPinged/pingCount	Yes — sanitizeCanary preserves webhookUrl/label/lastPinged/pingCount; the stored webhook is already https
Popup/inline edit forms	scalar fields, full passkey object on edit	Yes — complete PasskeyData passes sanitizePasskey
"Remove passkey" UX	passkey: undefined	Yes — explicit removal branch kept
ADD_ADDITIONAL_URL handler (messages.ts:510)	{ additionalUrls }	Untouched branch
SAVE_NOTE handler (messages.ts:542)	{ title, notes, modifiedAt }	Untouched branches (modifiedAt was already ignored by the whitelist)
Breach updater (messages.ts:839)	{ breach }	Untouched branch

Note on URL normalization: sanitizeCanary round-trips the webhook through new URL(...).toString(), so https://x.example.com persists as https://x.example.com/. This already happens on the add/restore paths; the patch path simply joins them.

messages.ts handler verification (no bypass)

UPDATE_ENTRY (messages.ts:478–501) has exactly one mutation path: vault.updateEntry(id, patch) → sanitizeEntryPatch (vault.ts:1836). There is no second entry point that skips the sanitizer. The Pro gate for new canaries (messages.ts:491–498) and the fire-time SSRF guard (isAllowedCanaryWebhookUrl, messages.ts:1568) remain as the outer gates.

Adjacent observation (no change made): vault.updateEntryDirect (vault.ts:1978) spreads a patch verbatim, but it has zero production callers (grep: only its definition; the sync path uses applyMergedRemoteEntries, which funnels through sanitizeRestoredEntry). Flagged for a future dead-code pass; out of scope for this finding.

Tests

vault.update-entry-sanitize.test.ts (in this folder) — 8 regression tests. Destination: src/background/vault.update-entry-sanitize.test.ts (imports are already in destination form).

Save message (integration)

fix: PCH-02-001 UPDATE_ENTRY patch routes canary/tags/passkey through existing sanitizers