@@ -69,7 +69,10 @@ async function getItem(rest: any, type: string, name: string) {
6969 const route = rest . getRoutes ( ) . find ( ( r : any ) => r . method === 'GET' && r . path === '/api/v1/meta/:type/:name' ) ;
7070 if ( ! route ) throw new Error ( 'meta/:type/:name route not registered' ) ;
7171 const res = makeRes ( ) ;
72- await route . handler ( { method : 'GET' , params : { type, name } , query : { } , body : { } } , res ) ;
72+ // `headers` is not optional dressing: the cached branch reads
73+ // `req.headers['if-none-match']`, so a request object without it would throw
74+ // its way into a 400 and read as "the gate denied" for the wrong reason.
75+ await route . handler ( { method : 'GET' , params : { type, name } , query : { } , body : { } , headers : { } } , res ) ;
7376 return res ;
7477}
7578
@@ -129,3 +132,161 @@ describe('the same spelling sensitivity on the other per-type gates', () => {
129132 expect ( names ( plural . body ) ) . toEqual ( names ( singular . body ) ) ;
130133 } ) ;
131134} ) ;
135+
136+ // ---------------------------------------------------------------------------
137+ // [#6241] The same gate, one branch further in: the CACHED read path.
138+ //
139+ // Everything above tests a protocol double with no `getMetaItemCached`, so the
140+ // single-item read always fell through to the uncached branch — the branch that
141+ // holds the §6.7 gate. A real deployment does not look like that: `enableCache`
142+ // defaults to `true` and the metadata protocol ships `getMetaItemCached`, so the
143+ // DEFAULT single-item read took the cached branch, whose entry condition
144+ // excluded `doc` / `book` by LITERAL comparison against the raw `:type` segment:
145+ //
146+ // … && req.params.type !== 'doc' && req.params.type !== 'book'
147+ //
148+ // `/meta/books/:name` is the canonical spelling (Prime Directive #3) and the
149+ // route serves it, so the plural read walked past the exclusion, took the cached
150+ // branch, and the audience gate never ran. Measured on the real `RestServer`
151+ // before the fix — one `{ permissionSet }`-gated book, one signed-in caller who
152+ // holds no set:
153+ //
154+ // singular "book" :: cachedCalls=0 status=[403] PERMISSION_DENIED
155+ // plural "books" :: cachedCalls=1 status=[] full gated body served
156+ //
157+ // That is #3984's defect recurring in the same file, and it is why the fix
158+ // normalizes once at the top of the handler instead of adding a third correctly
159+ // normalized comparison beside two wrong ones.
160+ //
161+ // The trade this pins: `docs` / `books` plural reads now leave the cached
162+ // branch, so they carry no ETag. Same trade #5881 made for `dashboard`, on a
163+ // harder reason — the comment above the exclusion has always said a shared ETag
164+ // over a per-caller-gated document leaks it across viewers, and
165+ // `getMetaItemCached` delegates to `getMetaItem`, so only the 304's saved bytes
166+ // are given up.
167+ // ---------------------------------------------------------------------------
168+ describe ( '#6241 — the cached branch cannot be spelled around either' , ( ) => {
169+ /** A doc the gated book claims by rule, plus one no book claims. */
170+ const ADMIN_DOC = { name : 'admin_runbook' , label : 'Runbook' } ;
171+ const OPEN_DOC = { name : 'intro' , label : 'Intro' } ;
172+ const CLAIMING_GATED_BOOK = {
173+ name : 'admin_guide' ,
174+ label : 'Admin Guide' ,
175+ audience : { permissionSet : 'crm_admin' } ,
176+ groups : [ { key : 'admin' , label : 'Admin' , include : 'admin_*' } ] ,
177+ } ;
178+ /** A type with no per-caller gate at all — the positive control's subject. */
179+ const VIEW_ITEM = { name : 'account_list' , label : 'Accounts' } ;
180+
181+ /**
182+ * The DEFAULT deployment shape: no `metadata` block at all (so `enableCache`
183+ * is its default `true`) and a protocol offering BOTH reads, so which branch
184+ * the handler picks is the thing under test rather than an artefact of a
185+ * double that only implements one.
186+ */
187+ function setupCached ( ) {
188+ const protocol : any = {
189+ getDiscovery : vi . fn ( ) . mockResolvedValue ( { version : 'v0' , routes : { data : '' , metadata : '' , ui : '' , auth : '/auth' } } ) ,
190+ getMetaTypes : vi . fn ( ) . mockResolvedValue ( [ ] ) ,
191+ getMetaItems : vi . fn ( async ( { type } : any ) => {
192+ const t = String ( type ?? '' ) ;
193+ if ( t === 'book' || t === 'books' ) return [ PUBLIC_BOOK , CLAIMING_GATED_BOOK ] ;
194+ if ( t === 'doc' || t === 'docs' ) return [ ADMIN_DOC , OPEN_DOC ] ;
195+ return [ ] ;
196+ } ) ,
197+ getMetaItem : vi . fn ( async ( { type, name } : any ) => {
198+ const all : any [ ] = [ PUBLIC_BOOK , CLAIMING_GATED_BOOK , ADMIN_DOC , OPEN_DOC , VIEW_ITEM ] ;
199+ const item = all . find ( ( i ) => i . name === name ) ;
200+ return item ? { type, name, item } : { type, name } ;
201+ } ) ,
202+ // Present and eligible — exactly what a default deployment has, and what
203+ // every test above this line was missing. It answers the UNFILTERED
204+ // document with an ETag over it, which is the leak the exclusion exists
205+ // to prevent.
206+ getMetaItemCached : vi . fn ( async ( { name } : any ) => ( {
207+ data : [ PUBLIC_BOOK , CLAIMING_GATED_BOOK , ADMIN_DOC , OPEN_DOC , VIEW_ITEM ]
208+ . find ( ( i ) => i . name === name ) ,
209+ etag : { value : 'etag-unfiltered' , weak : false } ,
210+ cacheControl : { directives : [ 'private' , 'no-cache' ] } ,
211+ notModified : false ,
212+ } ) ) ,
213+ findData : vi . fn ( ) . mockResolvedValue ( [ ] ) ,
214+ } ;
215+ const rest : any = new RestServer ( createMockServer ( ) as any , protocol , { api : { requireAuth : false } } as any ) ;
216+ // A signed-in caller who holds no permission set — the 403 case, not the
217+ // anonymous 401 one, so the pin cannot pass by accident on the auth gate.
218+ rest . resolveExecCtx = async ( ) => ( { userId : 'u1' } ) ;
219+ rest . securityServiceProvider = async ( ) => ( { resolvePermissionSetNames : async ( ) => [ ] } ) ;
220+ rest . registerRoutes ( ) ;
221+ return { rest, protocol } ;
222+ }
223+
224+ it ( 'a {permissionSet}-gated book is 403 on the PLURAL spelling, not 200' , async ( ) => {
225+ const { rest, protocol } = setupCached ( ) ;
226+ const res = await getItem ( rest , 'books' , 'admin_guide' ) ;
227+
228+ // Before the fix: 200 with `{ item: { audience: { permissionSet: … } } }`.
229+ expect ( res . statusCode ) . toBe ( 403 ) ;
230+ expect ( res . body ?. code ?? res . body ?. error ?. code ) . toBe ( 'PERMISSION_DENIED' ) ;
231+ // …and it got there by NOT taking the cached branch, which is the actual
232+ // mechanism — asserting only the status would leave a fix that gated the
233+ // cached body under an unfiltered ETag looking correct.
234+ expect ( protocol . getMetaItemCached ) . not . toHaveBeenCalled ( ) ;
235+ } ) ;
236+
237+ it ( 'the singular spelling keeps denying it — no regression on the path that worked' , async ( ) => {
238+ const { rest, protocol } = setupCached ( ) ;
239+ const res = await getItem ( rest , 'book' , 'admin_guide' ) ;
240+
241+ expect ( res . statusCode ) . toBe ( 403 ) ;
242+ expect ( protocol . getMetaItemCached ) . not . toHaveBeenCalled ( ) ;
243+ } ) ;
244+
245+ it ( 'a doc claimed only by the gated book is 403 on /meta/docs/:name too' , async ( ) => {
246+ // §6.7 effective audience: the union over the books claiming the doc. The
247+ // gated book's `include: admin_*` rule claims `admin_runbook`, so a
248+ // non-holder is denied — on either spelling.
249+ const { rest, protocol } = setupCached ( ) ;
250+ const plural = await getItem ( rest , 'docs' , 'admin_runbook' ) ;
251+ expect ( plural . statusCode ) . toBe ( 403 ) ;
252+ expect ( protocol . getMetaItemCached ) . not . toHaveBeenCalled ( ) ;
253+
254+ const singular = await getItem ( rest , 'doc' , 'admin_runbook' ) ;
255+ expect ( singular . statusCode ) . toBe ( 403 ) ;
256+ } ) ;
257+
258+ it ( 'an unclaimed doc still reads (org default) — the gate narrows, it does not close the surface' , async ( ) => {
259+ const { rest } = setupCached ( ) ;
260+ // `intro` is claimed by no book → effective audience `org` → a signed-in
261+ // caller may read it. Without this the three assertions above would be
262+ // satisfied by a fix that denied every doc/book read.
263+ expect ( ( await getItem ( rest , 'docs' , 'intro' ) ) . statusCode ) . toBe ( 200 ) ;
264+ expect ( ( await getItem ( rest , 'books' , 'manual' ) ) . statusCode ) . toBe ( 200 ) ;
265+ } ) ;
266+
267+ it ( 'positive control: a non-gated type still takes the cached branch, ETag and all' , async ( ) => {
268+ // The bypass is only correct if it bypasses exactly the gated types. A fix
269+ // that disabled the cache wholesale would satisfy every assertion above and
270+ // silently cost every other metadata read its validator.
271+ const { rest, protocol } = setupCached ( ) ;
272+ const res = await getItem ( rest , 'views' , 'account_list' ) ;
273+
274+ expect ( protocol . getMetaItemCached ) . toHaveBeenCalledTimes ( 1 ) ;
275+ expect ( protocol . getMetaItem ) . not . toHaveBeenCalled ( ) ;
276+ expect ( res . header . mock . calls . map ( ( c : any [ ] ) => c [ 0 ] ) ) . toContain ( 'ETag' ) ;
277+
278+ // …and the singular spelling of that same non-gated type, so the fix is
279+ // "normalize", not "move the doc/book hole onto some other type".
280+ const { rest : rest2 , protocol : protocol2 } = setupCached ( ) ;
281+ await getItem ( rest2 , 'view' , 'account_list' ) ;
282+ expect ( protocol2 . getMetaItemCached ) . toHaveBeenCalledTimes ( 1 ) ;
283+ } ) ;
284+
285+ it ( 'the price of the bypass, pinned rather than hidden: gated reads carry no ETag' , async ( ) => {
286+ const { rest } = setupCached ( ) ;
287+ const res = await getItem ( rest , 'books' , 'manual' ) ;
288+
289+ expect ( res . statusCode ) . toBe ( 200 ) ;
290+ expect ( res . header . mock . calls . map ( ( c : any [ ] ) => c [ 0 ] ) ) . not . toContain ( 'ETag' ) ;
291+ } ) ;
292+ } ) ;
0 commit comments