From b83911157157743fd5983c787255d243215cc6cf Mon Sep 17 00:00:00 2001 From: archipelago Date: Fri, 31 Jul 2026 06:49:16 -0400 Subject: [PATCH] fix(02-review): audit and set explicit persist on every useCachedResource call site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that persist is required (no default) on useCachedResource()/refresh(), every call site that previously relied on the implicit persist:true default needs an explicit decision. Full audit, decision rule: money/identity/ peer-identity payloads -> false; static/aggregate/non-identifying data -> true; ambiguous cases fail safe to false and are called out below. persist:false (financial / identity / peer-identity payload): - LightningChannelsPanel.vue: lnd.channels, lnd.closed-channels (open/closed Lightning channel balances — wallet data, same class as CR-01's lnd-info) - Cloud.vue: cloud.paid-items (carries paid_sats + purchase history), cloud.peer-nodes (PeerNode carries did/pubkey/onion) - Cloud.vue/PeerFiles.vue: cloud.my-files — not a clean money/identity/ peer-identity case, but a private per-user file listing; chosen false as the fail-safe default per the audit rule, flagged here for review - Credentials.vue: credentials.identities, credentials.list - Federation.vue: federation.nodes (FederatedNode carries did — matches Mesh.vue's already-persist:false federation.nodes decision) - FipsSeedAnchorsCard.vue: server.fips-seed-anchors (SeedAnchor carries npub) - Server.vue + FipsNetworkCard.vue: server.fips-summary corrected from persist:true to persist:false — this shared cache key's real fips.status response carries npub (this node's own FIPS identity key), which Server.vue's narrower local type didn't surface but FipsNetworkCard.vue's fuller FipsStatus type does; both call sites must agree since a mismatch trips the dev-only entry() persist-consistency warning. Found during this audit, not part of the originally-scoped call-site list — corrected as a same-class T-02-01 violation. serverTabCache.test.ts updated to match. persist:true (aggregate/status/public data, no identity or money): - AppDetails.vue: app-details:bitcoin-sync (block height/sync progress) - Cloud.vue: cloud.section-counts (bare per-section item counts); cloud.peer-browse (browsePeer()/loadCatalog()'s direct resources.refresh() calls now pass { persist: true } explicitly, matching the pre-existing decision already documented at peerBrowseEntry()) - Federation.vue: federation.dwn-status (sync status/counters only) - MarketplaceAppDetails.vue: app-details:versions (public catalog metadata) - Monitoring.vue: monitoring.current/history/alerts/alert-rules (system metrics and alert metadata only) - OpenWrtGateway.vue: server.openwrt-status (network/router status, matches sibling server.* resources) Co-Authored-By: Claude Opus 5 (1M context) --- neode-ui/src/components/LightningChannelsPanel.vue | 2 ++ neode-ui/src/views/AppDetails.vue | 1 + neode-ui/src/views/Cloud.vue | 10 +++++++++- neode-ui/src/views/Credentials.vue | 2 ++ neode-ui/src/views/Federation.vue | 2 ++ neode-ui/src/views/MarketplaceAppDetails.vue | 6 ++++-- neode-ui/src/views/Monitoring.vue | 4 ++++ neode-ui/src/views/PeerFiles.vue | 2 +- neode-ui/src/views/Server.vue | 13 +++++++++++-- neode-ui/src/views/__tests__/serverTabCache.test.ts | 10 ++++++++-- neode-ui/src/views/server/FipsNetworkCard.vue | 5 +++++ neode-ui/src/views/server/FipsSeedAnchorsCard.vue | 1 + neode-ui/src/views/server/OpenWrtGateway.vue | 1 + 13 files changed, 51 insertions(+), 8 deletions(-) diff --git a/neode-ui/src/components/LightningChannelsPanel.vue b/neode-ui/src/components/LightningChannelsPanel.vue index 6597aac8..b417788e 100644 --- a/neode-ui/src/components/LightningChannelsPanel.vue +++ b/neode-ui/src/components/LightningChannelsPanel.vue @@ -479,6 +479,7 @@ const channelsRes = useCachedResource({ total_outbound: result?.total_outbound || 0, } }, + persist: false, // open channel balances/capacity — wallet data (T-02-01) }) const closedRes = useCachedResource({ key: 'lnd.closed-channels', @@ -488,6 +489,7 @@ const closedRes = useCachedResource({ }) return closed?.channels || [] }, + persist: false, // closed channel settlement records — wallet data (T-02-01) }) const loading = computed(() => channelsRes.loadState.value === 'loading' || channelsRes.loadState.value === 'refreshing') diff --git a/neode-ui/src/views/AppDetails.vue b/neode-ui/src/views/AppDetails.vue index 6c11eb1f..04a82bba 100644 --- a/neode-ui/src/views/AppDetails.vue +++ b/neode-ui/src/views/AppDetails.vue @@ -177,6 +177,7 @@ const bitcoinSyncResource = useCachedResource<{ block_height: number; sync_progr timeout: 5000, }), ttlMs: 30_000, // install/health-state-shaped data, per plan default + persist: true, // public chain height/sync progress — no money amount or identity immediate: false, // kicked from onMounted, gated on needsBitcoinSync }) const bitcoinSyncPercent = computed(() => (bitcoinSyncResource.data.value?.sync_progress ?? 0) * 100) diff --git a/neode-ui/src/views/Cloud.vue b/neode-ui/src/views/Cloud.vue index e06894b9..6c4352d3 100644 --- a/neode-ui/src/views/Cloud.vue +++ b/neode-ui/src/views/Cloud.vue @@ -428,6 +428,7 @@ const countsResource = useCachedResource>({ key: 'cloud.section-counts', fetcher: fetchCounts, ttlMs: 30_000, + persist: true, // bare per-section item counts — no file names/content/identity immediate: false, // gated on fileBrowserRunning; kicked from onMounted/watch }) const sectionCounts = computed(() => countsResource.entry.data ?? {}) @@ -460,6 +461,7 @@ const paidResource = useCachedResource({ const res = await rpcClient.call<{ items: PaidItem[] }>({ method: 'content.owned-list', signal, dedup: true }) return (res.items || []).slice().reverse() }, + persist: false, // PaidItem carries paid_sats + purchase history — financial data (T-02-01) immediate: false, // loaded when the Paid tab is opened }) const paidItems = computed(() => paidResource.entry.data ?? []) @@ -517,6 +519,7 @@ const peersResource = useCachedResource({ return result?.nodes ?? [] }, ttlMs: 30_000, + persist: false, // PeerNode carries did/pubkey/onion — peer identity payload (T-02-01) immediate: false, // kicked from onMounted (keeps the legacy load order) }) const peerNodes = computed(() => peersResource.entry.data ?? []) @@ -632,6 +635,11 @@ const myFilesResource = useCachedResource({ key: 'cloud.my-files', fetcher: fetchMyFiles, ttlMs: 60_000, + // Fail-safe choice, not an obvious money/identity/peer-identity case: a + // flat listing of the user's own filenames/paths is private content, not + // squarely one of the three T-02-01 categories. Flagged for human review + // rather than defaulted to persist:true. + persist: false, immediate: false, // loaded when the My Files tab (or search) needs it }) const myFiles = computed(() => myFilesResource.entry.data ?? []) @@ -802,7 +810,7 @@ function browsePeer(peer: PeerNode): Promise { signal: browsePeerAborter.signal, }) return { items: res?.items ?? [], transport: res?.transport ?? null, latencyMs: Date.now() - t0 } - }) + }, { persist: true }) // matches peerBrowseEntry()'s explicit persist:true for this same key // A timed-out/failed peer resolves through resources.ts's own catch path // (loadState -> 'error', no throw here) — it never blocks or delays any // other peer's slot, and the UI already surfaces this silently (the diff --git a/neode-ui/src/views/Credentials.vue b/neode-ui/src/views/Credentials.vue index 84130045..06303a7b 100644 --- a/neode-ui/src/views/Credentials.vue +++ b/neode-ui/src/views/Credentials.vue @@ -239,6 +239,7 @@ const identitiesRes = useCachedResource({ }) return result?.identities || [] }, + persist: false, // identity records — identity payload (T-02-01) }) const credentialsRes = useCachedResource({ key: 'credentials.list', @@ -248,6 +249,7 @@ const credentialsRes = useCachedResource({ }) return result?.credentials || [] }, + persist: false, // credential material — identity payload (T-02-01) }) const identities = computed(() => identitiesRes.data.value ?? []) const credentials = computed(() => credentialsRes.data.value ?? []) diff --git a/neode-ui/src/views/Federation.vue b/neode-ui/src/views/Federation.vue index 9714857e..aed6d3bb 100644 --- a/neode-ui/src/views/Federation.vue +++ b/neode-ui/src/views/Federation.vue @@ -257,6 +257,7 @@ const syncStore = useSyncStore() const nodesRes = useCachedResource({ key: 'federation.nodes', fetcher: async () => (await rpcClient.federationListNodes()).nodes, + persist: false, // FederatedNode carries did/pubkey/onion — peer identity payload (T-02-01) }) const nodes = computed(() => nodesRes.data.value ?? []) const loading = computed(() => nodesRes.loadState.value === 'loading') @@ -331,6 +332,7 @@ const mapLinks = computed(() => { const dwnStatusRes = useCachedResource({ key: 'federation.dwn-status', fetcher: (signal) => rpcClient.call({ method: 'dwn.status', signal, dedup: true, maxRetries: 1 }), + persist: true, // sync status/counters only — no identity payload immediate: false, }) const dwnStatus = computed(() => dwnStatusRes.data.value) diff --git a/neode-ui/src/views/MarketplaceAppDetails.vue b/neode-ui/src/views/MarketplaceAppDetails.vue index 5e3da09a..eaf9497f 100644 --- a/neode-ui/src/views/MarketplaceAppDetails.vue +++ b/neode-ui/src/views/MarketplaceAppDetails.vue @@ -415,8 +415,9 @@ const appId = computed(() => route.params.id as string) // Keyed per marketplace app id (D-04). Catalog version metadata is // near-static, so this can take a longer TTL than the 30s default; it holds -// nothing credential/DID/wallet/tx-history-shaped so persist stays default -// (true). Note per 02-FINDINGS.md: this is the only one of MarketplaceAppDetails's +// nothing credential/DID/wallet/tx-history-shaped so persist:true is safe +// (explicit, not defaulted — CR-01 follow-up removed the implicit default). +// Note per 02-FINDINGS.md: this is the only one of MarketplaceAppDetails's // captured RPC calls that isn't confounded by the Home-tab-transit artifact — // getCurrentApp() is a synchronous store read (no RPC) and the bitcoin-prune // check below is a plain fetch(), not an rpcClient call. @@ -430,6 +431,7 @@ const versionsResource = useCachedResource({ timeout: 15000, }), ttlMs: 120_000, + persist: true, // package version list — public catalog metadata, no identity/money immediate: false, }) diff --git a/neode-ui/src/views/Monitoring.vue b/neode-ui/src/views/Monitoring.vue index 62094e30..527d7b47 100644 --- a/neode-ui/src/views/Monitoring.vue +++ b/neode-ui/src/views/Monitoring.vue @@ -303,6 +303,7 @@ const currentRes = useCachedResource({ if (!data || !('system' in data)) throw new Error('metrics not ready') return data }, + persist: true, // system/container/rpc metrics — no identity/money payload }) const historyRes = useCachedResource({ key: 'monitoring.history.minute60', @@ -313,6 +314,7 @@ const historyRes = useCachedResource({ }) return data?.data ?? [] }, + persist: true, // historical metric snapshots — no identity/money payload }) const alertsRes = useCachedResource({ key: 'monitoring.alerts', @@ -322,6 +324,7 @@ const alertsRes = useCachedResource({ }) return (data?.alerts ?? []).reverse() }, + persist: true, // alert metadata (kind/message/threshold) — no identity/money payload }) const alertRulesRes = useCachedResource({ key: 'monitoring.alert-rules', @@ -331,6 +334,7 @@ const alertRulesRes = useCachedResource({ }) return data?.rules ?? [] }, + persist: true, // static alert-rule config — no identity/money payload }) const current = computed(() => currentRes.data.value) const history = computed(() => historyRes.data.value ?? []) diff --git a/neode-ui/src/views/PeerFiles.vue b/neode-ui/src/views/PeerFiles.vue index 3cf92d8e..1d55cc77 100644 --- a/neode-ui/src/views/PeerFiles.vue +++ b/neode-ui/src/views/PeerFiles.vue @@ -901,7 +901,7 @@ function loadCatalog(): Promise { dedup: true, }) return { items: result?.items ?? [], transport: result?.transport ?? null, latencyMs: Date.now() - t0 } - }) + }, { persist: true }) // matches peerBrowseEntry()'s explicit persist:true for this same key } // Load visual previews for image and video items when catalog loads. diff --git a/neode-ui/src/views/Server.vue b/neode-ui/src/views/Server.vue index 656aec48..e85c0144 100644 --- a/neode-ui/src/views/Server.vue +++ b/neode-ui/src/views/Server.vue @@ -495,12 +495,21 @@ const networkRefreshing = computed(() => networkRes.loadState.value === 'refresh // TTL 60s (near-static: installed/service_active/key_present rarely change, // but authenticated_peer_count can drift, so this sits between the fast // 10s tier and Tor/VPN's 30s tier rather than a fully-static value). -// persist:true — key_present is a boolean flag, not the key material itself. +// persist:false (CR-01 follow-up audit correction) — this component's own +// local type only names installed/service_active/key_present/etc, but +// `fips.status` is the SAME RPC/cache key FipsNetworkCard.vue shares +// ("Shares `server.fips-summary`..."), and THAT component's fuller +// `FipsStatus` type shows the real response also carries `npub` — this +// node's own FIPS identity public key. Whichever of the two fetchers wins a +// given refresh writes the FULL raw response to sessionStorage regardless +// of either caller's narrower TS type, so this key must follow the same +// "own identity — never persist" rule as mesh.self-onion/mesh.self-did +// (T-02-01), not the "aggregate status only" rule its old comment assumed. const fipsSummaryRes = useCachedResource<{ installed: boolean; service_active: boolean; key_present: boolean; anchor_connected?: boolean; authenticated_peer_count?: number }>({ key: 'server.fips-summary', immediate: false, ttlMs: 60_000, - persist: true, + persist: false, fetcher: (signal) => rpcClient.call({ method: 'fips.status', signal, dedup: true, maxRetries: 1 }), }) const fipsSummary = computed(() => fipsSummaryRes.data.value) diff --git a/neode-ui/src/views/__tests__/serverTabCache.test.ts b/neode-ui/src/views/__tests__/serverTabCache.test.ts index 396ec6e5..edb3a4d0 100644 --- a/neode-ui/src/views/__tests__/serverTabCache.test.ts +++ b/neode-ui/src/views/__tests__/serverTabCache.test.ts @@ -232,7 +232,7 @@ describe('Server tab cache (Task 1): seven load groups', () => { wrapper.unmount() }) - it('groups carrying VPN peer identity or Tor onion addresses declare persist:false; non-identity groups may persist', async () => { + it('groups carrying VPN peer identity, Tor onion addresses, or this node\'s own FIPS identity key declare persist:false; non-identity groups may persist', async () => { const wrapper = mountServerHost() await flushPromises() @@ -246,7 +246,13 @@ describe('Server tab cache (Task 1): seven load groups', () => { expect(readSnapshot('server.vpn-peers')).toBeNull() // carries npub (peer identity) expect(readSnapshot('server.tor-services')).toBeNull() // carries onion_address expect(readSnapshot('server.network-summary')).not.toBeNull() // this node's own status - expect(readSnapshot('server.fips-summary')).not.toBeNull() + // CR-01 follow-up correction: `server.fips-summary` is shared with + // FipsNetworkCard.vue, whose fuller FipsStatus type shows the real + // `fips.status` response also carries `npub` — this node's own FIPS + // identity public key — so this key must be persist:false too (T-02-01), + // not persist:true as originally assumed when only the narrower + // installed/service_active/key_present fields were considered. + expect(readSnapshot('server.fips-summary')).toBeNull() expect(readSnapshot('server.interfaces')).not.toBeNull() expect(readSnapshot('server.disk-status')).not.toBeNull() diff --git a/neode-ui/src/views/server/FipsNetworkCard.vue b/neode-ui/src/views/server/FipsNetworkCard.vue index 612ac10c..b36d5dce 100644 --- a/neode-ui/src/views/server/FipsNetworkCard.vue +++ b/neode-ui/src/views/server/FipsNetworkCard.vue @@ -139,9 +139,14 @@ interface FipsStatus { // Shares `server.fips-summary` with the Local Network card's FIPS row, so // both paint from the same cache instantly on revisit and never disagree. +// persist:false (T-02-01) — the response carries `npub`, this node's own +// FIPS identity public key; must match Server.vue's persist decision for +// this same key exactly (a mismatch would trip the dev-only entry() warning +// and leave it ambiguous which decision actually governs the shared cache). const statusRes = useCachedResource({ key: 'server.fips-summary', ttlMs: 15_000, + persist: false, fetcher: (signal) => rpcClient.call({ method: 'fips.status', signal, dedup: true, maxRetries: 1 }), }) const status = computed(() => statusRes.data.value ?? { diff --git a/neode-ui/src/views/server/FipsSeedAnchorsCard.vue b/neode-ui/src/views/server/FipsSeedAnchorsCard.vue index e1609cae..2f9f75c2 100644 --- a/neode-ui/src/views/server/FipsSeedAnchorsCard.vue +++ b/neode-ui/src/views/server/FipsSeedAnchorsCard.vue @@ -116,6 +116,7 @@ const anchorsRes = useCachedResource({ }) return res.seed_anchors }, + persist: false, // SeedAnchor carries an npub (peer identity key material) — T-02-01 }) const anchors = computed(() => anchorsRes.data.value ?? []) const adding = ref(false) diff --git a/neode-ui/src/views/server/OpenWrtGateway.vue b/neode-ui/src/views/server/OpenWrtGateway.vue index cda0374b..ef8e9597 100644 --- a/neode-ui/src/views/server/OpenWrtGateway.vue +++ b/neode-ui/src/views/server/OpenWrtGateway.vue @@ -93,6 +93,7 @@ const routerResource = useCachedResource({ maxRetries: 1, }), ttlMs: 30_000, + persist: true, // router/network status (host/uptime/wifi/wan) — no identity/money payload immediate: false, // initial fetch is gated explicitly in onMounted below }) const status = computed(() => routerResource.data.value)