fix(secure-context): full audit — DestructiveConfirmModal.expectedTokenOverride + bulk-delete + min-dim use backend-computed tokens

Operator-flagged 2026-05-27: walk the whole project for the same shape as the min-dim Delete-button silent failure (crypto.subtle TypeError on plain HTTP). FC runs over plain HTTP per the homelab posture; Secure-Context-gated browser APIs are undefined on the production origin. **Audit results across `frontend/src/`:** crypto.subtle.digest — 2 sites: - MinDimensionCard (fixed 2026-05-27) - BulkEditorPanel (THIS FIX) navigator.clipboard — 1 site, already guarded: - utils/clipboard.js writeText with execCommand fallback serviceWorker / mediaDevices / Push / Web USB|HID|Bluetooth|Serial / cookieStore / queryLocalFonts / WebAuthn / geolocation — NOT USED, nothing to fix Extension scripts (background.js) use crypto.subtle but run from moz-extension:// which IS a Secure Context — left as-is. **BulkEditorPanel double bug** The bulk-delete UI on the gallery selection had been broken since FC-3k shipped, in two ways: 1. `crypto.subtle.digest` swallowed TypeError on plain HTTP — modal never opened. Same symptom as min-dim. 2. Even on HTTPS, the modal's `kind="images-selection"` produced `delete-images-selection-<sha8>` while the backend expected `delete-images-<sha8>`. The two would never match. Fix: - Backend `/api/admin/images/bulk-delete` dry-run response now returns `confirm_token` (the canonical `delete-images-<sha8>` string). Integration test `test_bulk_delete_dry_run_returns_counts` pinned to assert the new field. - DestructiveConfirmModal gains an `expectedTokenOverride` prop. When set, it bypasses the `${action}-${kind}-${runId}` formula and uses the explicit string. This decouples the UI label (`kind`) from the wire-format token (server-provided), so future endpoints can use a kind-specific label without their kind name leaking into the token. - BulkEditorPanel passes `:expected-token-override="bulkProjected?.confirm_token"` — no client-side crypto, no kind-prefix mismatch. - MinDimensionCard refactored to the same explicit pattern (was slicing the 8-char suffix off the backend's token and passing it through `runId`; now passes the full backend token via `expected-token-override` directly). Cleaner; one source of truth. **Banked memory** `feedback_no_secure_context_apis.md` documents the full table of Secure-Context-gated APIs, which ones FC currently uses, and how each is handled. Indexed in MEMORY.md. Sites for the audit also listed in the memory for future drift-checking. No other Secure-Context-gated APIs found in `frontend/src/`. The same shape won't recur unless someone adds a new dependency on one — at which point the banked memory should fire.
2026-05-27 21:17:40 -04:00
parent 12be188ada
commit df6d89cb59
5 changed files with 49 additions and 36 deletions
@@ -97,11 +97,19 @@ async def images_bulk_delete():
            )
        )

-    if dry_run:
-        return jsonify(projected)
-
    sha8 = _bulk_image_confirm_token(image_ids)
    expected = f"delete-images-{sha8}"
+
+    if dry_run:
+        # Hand the canonical Tier-C confirm token back with the
+        # projection so the frontend doesn't have to recompute SHA-256
+        # client-side via crypto.subtle (Secure-Context-gated,
+        # undefined on plain-HTTP origins per the homelab posture).
+        # Operator-flagged 2026-05-27.
+        projected["confirm_token"] = expected
+        return jsonify(projected)
+
+
    if supplied_confirm != expected:
        return _bad(
            "confirm_mismatch",
@@ -52,8 +52,8 @@
      v-model="showModal"
      action="delete"
      kind="min-dim"
-      :run-id="tokenSuffix"
      tier="C"
+      :expected-token-override="preview?.confirm_token || ''"
      :projected-counts="projectedCounts"
      :description="`Width < ${minW} OR height < ${minH}`"
      @confirm="onConfirmedDelete"
@@ -62,11 +62,19 @@
 </template>

 <script setup>
-import { computed, onMounted, ref } from 'vue'
+import { onMounted, ref } from 'vue'

 import DestructiveConfirmModal from '../modal/DestructiveConfirmModal.vue'
 import { useCleanupStore } from '../../stores/cleanup.js'

+// Backend's preview response hands the full Tier-C confirm token back
+// as `confirm_token` (e.g. `delete-min-dim-1a2b3c4d`); passed straight
+// to the modal via `expected-token-override`. We previously
+// reconstructed via Web Crypto's SHA-256, but `crypto.subtle` is
+// Secure-Context-gated and undefined on plain-HTTP origins, so the
+// Delete button silently swallowed the TypeError. Operator-flagged
+// 2026-05-27.
+
 const store = useCleanupStore()
 const minW = ref(0)
 const minH = ref(0)
@@ -75,21 +83,6 @@ const busy = ref(false)
 const showModal = ref(false)
 const projectedCounts = ref({})

-// The backend hands the full Tier-C confirm token back with the
-// preview response (e.g. `delete-min-dim-1a2b3c4d`). We previously
-// reconstructed it client-side via Web Crypto's SHA-256, but
-// `crypto.subtle` is undefined on plain-HTTP origins (homelab posture)
-// and the Delete-button click handler was silently swallowing the
-// TypeError. Letting the backend own the token is also the single
-// source of truth — frontend just splits off the 8-char suffix to
-// feed DestructiveConfirmModal's `runId` slot (the modal templates
-// the prefix `delete-min-dim-` itself from action+kind).
-const tokenSuffix = computed(() => {
-  const full = preview.value?.confirm_token ?? ''
-  // `delete-min-dim-1a2b3c4d` → `1a2b3c4d`
-  return full.startsWith('delete-min-dim-') ? full.slice('delete-min-dim-'.length) : ''
-})
-
 onMounted(async () => {
  await store.loadDefaults()
  minW.value = store.defaults.min_width
@@ -74,7 +74,7 @@
      v-model="deleteModalOpen"
      action="delete"
      kind="images-selection"
-      :run-id="bulkToken"
+      :expected-token-override="bulkProjected?.confirm_token || ''"
      tier="C"
      :projected-counts="bulkProjectedCounts"
      :description="bulkDescription"
@@ -130,7 +130,6 @@ watch(() => sel.order.length, () => {
 const deleting = ref(false)
 const deleteModalOpen = ref(false)
 const bulkProjected = ref(null)
-const bulkToken = ref('')

 const bulkProjectedCounts = computed(() => bulkProjected.value
  ? {
@@ -147,24 +146,22 @@ const bulkDescription = computed(
    : '',
 )

-async function _computeSha8(ids) {
-  const canon = [...ids].sort((a, b) => a - b).join(',')
-  const buf = new TextEncoder().encode(canon)
-  const hashBuf = await crypto.subtle.digest('SHA-256', buf)
-  const bytes = new Uint8Array(hashBuf)
-  let hex = ''
-  for (let i = 0; i < 4; i++) {
-    hex += bytes[i].toString(16).padStart(2, '0')
-  }
-  return hex
-}
+// The dry-run response hands the canonical Tier-C confirm token back
+// as `confirm_token` (e.g. `delete-images-1a2b3c4d`), passed straight
+// to the modal via `expected-token-override`. We used to compute the
+// hash client-side via `crypto.subtle.digest`, but (1) that's
+// Secure-Context-gated and undefined on plain-HTTP origins
+// (homelab posture), so the click silently threw TypeError and the
+// modal never opened, and (2) the modal's `kind="images-selection"`
+// produced `delete-images-selection-<sha8>` while the backend
+// expected `delete-images-<sha8>` — so it never would have worked
+// even on HTTPS. Operator-flagged 2026-05-27.

 async function onDeleteClick() {
  if (!sel.order.length) return
  deleting.value = true
  try {
    bulkProjected.value = await adminStore.projectBulkImageDelete(sel.order)
-    bulkToken.value = await _computeSha8(sel.order)
    deleteModalOpen.value = true
  } finally {
    deleting.value = false
@@ -58,17 +58,27 @@ import { computed, ref, watch } from 'vue'
 const props = defineProps({
  modelValue: { type: Boolean, required: true },
  action: { type: String, required: true },          // 'restore' | 'delete'
-  kind: { type: String, required: true },            // 'db' | 'images' | 'artist' | 'tag' | 'images-selection'
+  kind: { type: String, required: true },            // 'db' | 'images' | 'artist' | 'tag' | 'images-selection' | 'audit' | 'min-dim'
  runId: { type: [Number, String], default: '' },    // numeric id or sha8 string
  description: { type: String, default: '' },
  tier: { type: String, default: 'C' },              // 'B' | 'C'
  projectedCounts: { type: Object, default: null },
+  // Override the `${action}-${kind}-${runId}` token formula. Use when
+  // the backend computes the canonical confirm token (e.g. bulk-delete
+  // and min-dim cleanup both return `confirm_token` from their dry-run
+  // endpoints) and the UI's kind/runId would otherwise produce a
+  // mismatched string. Operator-flagged 2026-05-27 after the
+  // BulkEditor's kind="images-selection" produced
+  // `delete-images-selection-<sha8>` while the backend expected
+  // `delete-images-<sha8>`.
+  expectedTokenOverride: { type: String, default: '' },
 })
 const emit = defineEmits(['update:modelValue', 'confirm'])

 const typed = ref('')
 const expectedToken = computed(
-  () => `${props.action}-${props.kind}-${props.runId}`,
+  () => props.expectedTokenOverride
+    || `${props.action}-${props.kind}-${props.runId}`,
 )
 const titleVerb = computed(
  () => props.action === 'restore' ? 'Restore' : 'Delete',
@@ -140,6 +140,11 @@ async def test_bulk_delete_dry_run_returns_counts(client, db, tmp_path):
    assert body["images_found"] == 2
    assert body["bytes_on_disk"] == 30
    assert body["missing_ids"] == [9_999_999]
+    # Dry-run hands the canonical Tier-C confirm token back so the
+    # frontend doesn't recompute SHA-256 client-side (crypto.subtle
+    # is Secure-Context-gated; FC runs over plain HTTP).
+    assert body["confirm_token"].startswith("delete-images-")
+    assert len(body["confirm_token"]) == len("delete-images-") + 8


@pytest.mark.asyncio