From 2f57ffd788fbd8232162ebf85f3f39fa344ae44b Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Sat, 29 Aug 2026 07:22:51 +0200 Subject: [PATCH] fix(walkthrough): withhold the tour until the vault is unlocked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The walkthrough merge broke `vault-unlock.spec.ts`: a locked vault issued `GET /api/preferences/walkthrough_completed_version`, and that spec asserts — on the wire, not the DOM — that a locked vault requests NO Keepiq API endpoint. It is an allowlist that fails closed, precisely so a new request behind the lock screen is a deliberate act with a reason. The router guard could not have caught this, and says so: it works by refusing to resolve a route, so it only covers what mounts inside the ``. CnAppRoot reads `manifest.walkthrough` and fetches the tour's completion preference when the SHELL mounts, before any route resolves — a shell-level sibling, which that guard's own note warns needs its own gating. So the manifest handed to the shell now omits `walkthrough` while locked. Withheld, not disabled: `isLocked` is read inside the render function, so the tour returns the moment the vault is unlocked and still runs on a user's first unlocked visit. Drawing a product tour over a locked vault would be wrong regardless of the request. Fails closed like the guard beside it — only an explicit `false` unlocks — so a store that failed to initialise withholds the tour rather than shipping it. Both properties are mutation-verified: reading the check as `!store?.isLocked` fails the fail-closed test, and removing the withholding fails the first. --- src/main.js | 8 +++-- src/router/guards.js | 45 +++++++++++++++++++++++++++ tests/router/guards.spec.js | 61 ++++++++++++++++++++++++++++++++++--- 3 files changed, 108 insertions(+), 6 deletions(-) diff --git a/src/main.js b/src/main.js index 5a9ba6d5..4b95ba04 100644 --- a/src/main.js +++ b/src/main.js @@ -43,7 +43,7 @@ import bundledManifest from './manifest.json' import menuLayout from './menu-layout.json' import pinia from './pinia.js' import registry from './registry.js' -import { createVaultGuard } from './router/guards.js' +import { createVaultGuard, manifestForLockState } from './router/guards.js' import { useSessionStore } from './store/modules/session.js' // Library CSS — must be explicit import (webpack tree-shakes side-effect imports from aliased packages) @@ -192,9 +192,13 @@ const customComponentsProp = Object.fromEntries( // Create and mount the app immediately so the shell renders. const app = createApp({ + // `manifestForLockState` withholds the walkthrough while the vault is + // locked. Reading `isLocked` here makes it a render dependency, so the tour + // reappears the moment the vault is unlocked — see the note on that helper + // for why the router guard above cannot cover this. render: () => h(App, { - manifest: mergedManifest, + manifest: manifestForLockState(mergedManifest, useSessionStore(pinia)), customComponents: customComponentsProp, pageTypes: pageTypesProp, registry: registryProp, diff --git a/src/router/guards.js b/src/router/guards.js index 2d47fba1..74756eb7 100644 --- a/src/router/guards.js +++ b/src/router/guards.js @@ -222,3 +222,48 @@ export function createVaultGuard(getSessionStore) { next() } } + +/** + * The manifest as the SHELL should see it for the current lock state. + * + * 🔴 THE VAULT GUARD ABOVE DOES NOT COVER THIS. That guard works by refusing + * to resolve a route, so it only reaches what mounts inside the + * ``. The walkthrough is a shell-level SIBLING of it: CnAppRoot + * reads `manifest.walkthrough` and fetches the tour's completion preference + * when the shell mounts, before any route resolves. That put + * `GET /api/preferences/walkthrough_completed_version` on the wire behind the + * lock screen and broke the invariant that a locked vault issues no Keepiq API + * request at all — the assertion that exists because a lock screen which is a + * redirect rather than a gate leaks the real inventory. + * + * Withheld, not disabled: the shell re-renders when `isLocked` flips, so the + * tour is offered on the first UNLOCKED visit. A product tour drawn over a + * locked vault would be the wrong behaviour regardless of the request. + * + * Fail closed, exactly as `createVaultGuard` does: only an explicit `false` + * counts as unlocked, so a store that failed to initialise withholds the tour + * rather than shipping it. + * + * @param {object} manifest The bundled manifest. + * @param {object} store The session store (with `isLocked`). + * @return {object} The manifest, without `walkthrough` while locked. + * @spec openspec/specs/encryption-suites/spec.md#requirement-session-mechanism + */ +export function manifestForLockState(manifest, store) { + if (store?.isLocked === false) { + return manifest + } + + if ( + manifest === null + || manifest === undefined + || manifest.walkthrough === undefined + ) { + return manifest + } + + const withoutWalkthrough = { ...manifest } + delete withoutWalkthrough.walkthrough + + return withoutWalkthrough +} diff --git a/tests/router/guards.spec.js b/tests/router/guards.spec.js index 65cfe5f3..3cfa7847 100644 --- a/tests/router/guards.spec.js +++ b/tests/router/guards.spec.js @@ -12,16 +12,17 @@ * @spec openspec/specs/encryption-suites/spec.md#requirement-session-mechanism */ -import { describe, it, expect, vi } from 'vitest' +import { describe, expect, it, vi } from 'vitest' +import manifest from '../../src/manifest.json' import { - LOCK_ROUTE_NAME, - PUBLIC_ROUTE_NAMES, createVaultGuard, handleLockTransition, isPublicRoute, isPublicSurface, + LOCK_ROUTE_NAME, + manifestForLockState, + PUBLIC_ROUTE_NAMES, } from '../../src/router/guards.js' -import manifest from '../../src/manifest.json' /** * Build a guard plus a spy `next`, over a session store of the given state. @@ -416,3 +417,55 @@ describe('isPublicSurface', () => { ).toBe(true) }) }) + +describe('manifestForLockState', () => { + const MANIFEST = { + version: 1, + pages: [], + walkthrough: { enabled: true, tours: [] }, + } + + it('withholds the walkthrough while the vault is locked', () => { + // CnAppRoot fetches the tour's completion preference as soon as it sees + // `manifest.walkthrough`, and it mounts OUTSIDE the router guard — so + // this is the only thing keeping that request off the wire behind the + // lock screen. + const shown = manifestForLockState(MANIFEST, { isLocked: true }) + + expect(shown.walkthrough).toBeUndefined() + expect(shown.pages).toBe(MANIFEST.pages) + }) + + it('offers the walkthrough once the vault is unlocked', () => { + // Withheld, not disabled: a first-visit tour must still run on a user's + // first UNLOCKED visit. + expect(manifestForLockState(MANIFEST, { isLocked: false })).toBe(MANIFEST) + }) + + it('fails closed when the store is missing or its flag is not a boolean', () => { + // Same posture as createVaultGuard: only an explicit `false` unlocks. + // A store that failed to initialise must withhold the tour, not ship it. + for (const store of [ + undefined, + null, + {}, + { isLocked: undefined }, + { isLocked: 'no' }, + { isLocked: 0 }, + ]) { + expect(manifestForLockState(MANIFEST, store).walkthrough).toBeUndefined() + } + }) + + it('leaves a manifest without a walkthrough untouched', () => { + const plain = { version: 1, pages: [] } + + expect(manifestForLockState(plain, { isLocked: true })).toBe(plain) + }) + + it('does not mutate the manifest it was given', () => { + manifestForLockState(MANIFEST, { isLocked: true }) + + expect(MANIFEST.walkthrough).toBeDefined() + }) +})