2
0
Эх сурвалжийг харах

fix(share-link): disable attachment-refs directives on share link pages

Extend the share-link guard (added for $lsx() in the previous commit) to
the attachment-refs directives: ref / refimg / refs / refsimg / gallery.

On a share link page these directives unconditionally fetched
/_api/attachment-refs/*, which returns 403 for anonymous viewers (the
/refs route requires login and has no certifySharedPage middleware) and,
behind a reverse proxy that exempts /share/ but not /_api/, triggers a
Basic auth challenge (#11263).

- renderer service (refs.ts): inject isSharedPage from rendererConfig
  into each ref element and allow it through sanitization, mirroring lsx
- AttachmentRefsDisabled: shared placeholder rendered in place of the
  directive substance on share link pages (LsxDisabled equivalent)
- ref/refimg/refs/refsimg/gallery: render the placeholder when
  isSharedPage so no useSWRx* request is issued; route the Immutable
  variants through the guarded wrapper
- renderer.tsx: thread config.isSharedPage into the refs rehype plugin
  at all three generators

Tests (TDD): rehype plugin injection (refs.spec.ts) and the no-request
contract for every directive (share-link-guard.spec.tsx). Route client
*.spec.tsx to a DOM environment in vitest.config.ts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Yuki Takei 2 өдөр өмнө
parent
commit
81fb6ddeb4

+ 12 - 3
apps/app/src/client/services/renderer/renderer.tsx

@@ -112,7 +112,10 @@ export const generateViewOptions = (
       lsxGrowiDirective.rehypePlugin,
       { pagePath, isSharedPage: config.isSharedPage },
     ],
-    [refsGrowiDirective.rehypePlugin, { pagePath }],
+    [
+      refsGrowiDirective.rehypePlugin,
+      { pagePath, isSharedPage: config.isSharedPage },
+    ],
     rehypeSanitizePlugin,
     katex,
     [relocateToc.rehypePluginStore, { storeTocNode }],
@@ -238,7 +241,10 @@ export const generateSimpleViewOptions = (
       lsxGrowiDirective.rehypePlugin,
       { pagePath, isSharedPage: config.isSharedPage },
     ],
-    [refsGrowiDirective.rehypePlugin, { pagePath }],
+    [
+      refsGrowiDirective.rehypePlugin,
+      { pagePath, isSharedPage: config.isSharedPage },
+    ],
     [keywordHighlighter.rehypePlugin, { keywords: highlightKeywords }],
     rehypeSanitizePlugin,
     katex,
@@ -342,7 +348,10 @@ export const generatePreviewOptions = (
       lsxGrowiDirective.rehypePlugin,
       { pagePath, isSharedPage: config.isSharedPage },
     ],
-    [refsGrowiDirective.rehypePlugin, { pagePath }],
+    [
+      refsGrowiDirective.rehypePlugin,
+      { pagePath, isSharedPage: config.isSharedPage },
+    ],
     addLineNumberAttribute.rehypePlugin,
     rehypeSanitizePlugin,
     katex,

+ 30 - 0
packages/remark-attachment-refs/src/client/components/AttachmentRefsDisabled.tsx

@@ -0,0 +1,30 @@
+import React, { type JSX } from 'react';
+
+type Props = {
+  /** The directive name to show in the message (e.g. 'ref', 'refs', 'gallery') */
+  name: string;
+};
+
+/**
+ * Placeholder rendered in place of a ref/refs directive on a share link page.
+ *
+ * Attachment ref directives fetch from `/_api/attachment-refs/*`, which is not
+ * accessible to anonymous share-link viewers. Rendering this instead of the
+ * substance avoids issuing those unauthenticated requests (see issue #11263).
+ */
+export const AttachmentRefsDisabled = React.memo(
+  ({ name }: Props): JSX.Element => {
+    return (
+      <div className="text-muted">
+        <span
+          className="material-symbols-outlined fs-5 me-1"
+          aria-hidden="true"
+        >
+          info
+        </span>
+        <small>{name} is not available on the share link page</small>
+      </div>
+    );
+  },
+);
+AttachmentRefsDisabled.displayName = 'AttachmentRefsDisabled';

+ 5 - 5
packages/remark-attachment-refs/src/client/components/Gallery.tsx

@@ -1,5 +1,6 @@
 import React, { type JSX } from 'react';
 
+import { AttachmentRefsDisabled } from './AttachmentRefsDisabled';
 import type { Props } from './RefsImg';
 import { RefsImgSubstance } from './RefsImg';
 
@@ -7,6 +8,9 @@ const gridDefault = 'col-4';
 const gridGapDefault = '1px';
 
 export const Gallery = React.memo((props: Props): JSX.Element => {
+  if (props.isSharedPage) {
+    return <AttachmentRefsDisabled name="gallery" />;
+  }
   const grid = props.grid || gridDefault;
   const gridGap = props.gridGap || gridGapDefault;
   return <RefsImgSubstance grid={grid} gridGap={gridGap} {...props} />;
@@ -14,11 +18,7 @@ export const Gallery = React.memo((props: Props): JSX.Element => {
 
 export const GalleryImmutable = React.memo(
   (props: Omit<Props, 'isImmutable'>): JSX.Element => {
-    const grid = props.grid || gridDefault;
-    const gridGap = props.gridGap || gridGapDefault;
-    return (
-      <RefsImgSubstance grid={grid} gridGap={gridGap} {...props} isImmutable />
-    );
+    return <Gallery {...props} isImmutable />;
   },
 );
 

+ 6 - 1
packages/remark-attachment-refs/src/client/components/Ref.tsx

@@ -2,12 +2,14 @@ import React, { type JSX, useMemo } from 'react';
 
 import { useSWRxRef } from '../stores/refs';
 import { AttachmentList } from './AttachmentList';
+import { AttachmentRefsDisabled } from './AttachmentRefsDisabled';
 import { RefsContext } from './util/refs-context';
 
 type Props = {
   fileNameOrId: string;
   pagePath: string;
   isImmutable?: boolean;
+  isSharedPage?: boolean;
 };
 
 const RefSubstance = React.memo(
@@ -35,12 +37,15 @@ const RefSubstance = React.memo(
 );
 
 export const Ref = React.memo((props: Props): JSX.Element => {
+  if (props.isSharedPage) {
+    return <AttachmentRefsDisabled name="ref" />;
+  }
   return <RefSubstance {...props} />;
 });
 
 export const RefImmutable = React.memo(
   (props: Omit<Props, 'isImmutable'>): JSX.Element => {
-    return <RefSubstance {...props} isImmutable />;
+    return <Ref {...props} isImmutable />;
   },
 );
 

+ 6 - 1
packages/remark-attachment-refs/src/client/components/RefImg.tsx

@@ -2,6 +2,7 @@ import React, { type JSX, useMemo } from 'react';
 
 import { useSWRxRef } from '../stores/refs';
 import { AttachmentList } from './AttachmentList';
+import { AttachmentRefsDisabled } from './AttachmentRefsDisabled';
 import { RefsContext } from './util/refs-context';
 
 type Props = {
@@ -14,6 +15,7 @@ type Props = {
   alt?: string;
 
   isImmutable?: boolean;
+  isSharedPage?: boolean;
 };
 
 const RefImgSubstance = React.memo(
@@ -58,12 +60,15 @@ const RefImgSubstance = React.memo(
 );
 
 export const RefImg = React.memo((props: Props): JSX.Element => {
+  if (props.isSharedPage) {
+    return <AttachmentRefsDisabled name="refimg" />;
+  }
   return <RefImgSubstance {...props} />;
 });
 
 export const RefImgImmutable = React.memo(
   (props: Omit<Props, 'isImmutable'>): JSX.Element => {
-    return <RefImgSubstance {...props} isImmutable />;
+    return <RefImg {...props} isImmutable />;
   },
 );
 

+ 6 - 1
packages/remark-attachment-refs/src/client/components/Refs.tsx

@@ -2,6 +2,7 @@ import React, { type JSX, useMemo } from 'react';
 
 import { useSWRxRefs } from '../stores/refs';
 import { AttachmentList } from './AttachmentList';
+import { AttachmentRefsDisabled } from './AttachmentRefsDisabled';
 import { RefsContext } from './util/refs-context';
 
 type Props = {
@@ -11,6 +12,7 @@ type Props = {
   regexp?: string;
 
   isImmutable?: boolean;
+  isSharedPage?: boolean;
 };
 
 const RefsSubstance = React.memo(
@@ -51,12 +53,15 @@ const RefsSubstance = React.memo(
 );
 
 export const Refs = React.memo((props: Props): JSX.Element => {
+  if (props.isSharedPage) {
+    return <AttachmentRefsDisabled name="refs" />;
+  }
   return <RefsSubstance {...props} />;
 });
 
 export const RefsImmutable = React.memo(
   (props: Omit<Props, 'isImmutable'>): JSX.Element => {
-    return <RefsSubstance {...props} isImmutable />;
+    return <Refs {...props} isImmutable />;
   },
 );
 

+ 6 - 1
packages/remark-attachment-refs/src/client/components/RefsImg.tsx

@@ -2,6 +2,7 @@ import React, { type JSX, useMemo } from 'react';
 
 import { useSWRxRefs } from '../stores/refs';
 import { AttachmentList } from './AttachmentList';
+import { AttachmentRefsDisabled } from './AttachmentRefsDisabled';
 import { RefsContext } from './util/refs-context';
 
 export type Props = {
@@ -19,6 +20,7 @@ export type Props = {
   noCarousel?: string;
 
   isImmutable?: boolean;
+  isSharedPage?: boolean;
 };
 
 export const RefsImgSubstance = React.memo(
@@ -109,12 +111,15 @@ export const RefsImgSubstance = React.memo(
 );
 
 export const RefsImg = React.memo((props: Props): JSX.Element => {
+  if (props.isSharedPage) {
+    return <AttachmentRefsDisabled name="refsimg" />;
+  }
   return <RefsImgSubstance {...props} />;
 });
 
 export const RefsImgImmutable = React.memo(
   (props: Omit<Props, 'isImmutable'>): JSX.Element => {
-    return <RefsImgSubstance {...props} isImmutable />;
+    return <RefsImg {...props} isImmutable />;
   },
 );
 

+ 69 - 0
packages/remark-attachment-refs/src/client/components/share-link-guard.spec.tsx

@@ -0,0 +1,69 @@
+import { render, screen } from '@testing-library/react';
+
+import { useSWRxRef, useSWRxRefs } from '../stores/refs';
+import { Gallery } from './Gallery';
+import { Ref } from './Ref';
+import { RefImg } from './RefImg';
+import { Refs } from './Refs';
+import { RefsImg } from './RefsImg';
+
+// The contract under test is "no request is issued on a share link page".
+// Mock the data-fetching hooks so we can assert they are never called,
+// and stub AttachmentList so the test does not pull in @growi/ui.
+vi.mock('../stores/refs', () => ({
+  useSWRxRef: vi.fn(() => ({
+    data: undefined,
+    error: undefined,
+    isLoading: false,
+  })),
+  useSWRxRefs: vi.fn(() => ({ data: [], error: undefined, isLoading: false })),
+}));
+
+vi.mock('./AttachmentList', () => ({
+  AttachmentList: () => <div data-testid="attachment-list" />,
+}));
+
+const DISABLED_MESSAGE = /not available on the share link page/i;
+
+describe('attachment-refs directives on a share link page', () => {
+  describe('when isSharedPage is true', () => {
+    it.each`
+      label        | mount
+      ${'ref'}     | ${() => render(<Ref pagePath="/foo" fileNameOrId="a.png" isSharedPage />)}
+      ${'refimg'}  | ${() => render(<RefImg pagePath="/foo" fileNameOrId="a.png" isSharedPage />)}
+      ${'refs'}    | ${() => render(<Refs pagePath="/foo" isSharedPage />)}
+      ${'refsimg'} | ${() => render(<RefsImg pagePath="/foo" isSharedPage />)}
+      ${'gallery'} | ${() => render(<Gallery pagePath="/foo" isSharedPage />)}
+    `(
+      'shows the disabled message and issues no request for $label',
+      ({ mount }: { mount: () => void }) => {
+        mount();
+
+        expect(screen.queryByText(DISABLED_MESSAGE)).not.toBeNull();
+        expect(screen.queryByTestId('attachment-list')).toBeNull();
+        expect(useSWRxRef).not.toHaveBeenCalled();
+        expect(useSWRxRefs).not.toHaveBeenCalled();
+      },
+    );
+  });
+
+  describe('when isSharedPage is not set', () => {
+    it.each`
+      label        | mount                                                            | hook
+      ${'ref'}     | ${() => render(<Ref pagePath="/foo" fileNameOrId="a.png" />)}    | ${useSWRxRef}
+      ${'refimg'}  | ${() => render(<RefImg pagePath="/foo" fileNameOrId="a.png" />)} | ${useSWRxRef}
+      ${'refs'}    | ${() => render(<Refs pagePath="/foo" />)}                        | ${useSWRxRefs}
+      ${'refsimg'} | ${() => render(<RefsImg pagePath="/foo" />)}                     | ${useSWRxRefs}
+      ${'gallery'} | ${() => render(<Gallery pagePath="/foo" />)}                     | ${useSWRxRefs}
+    `(
+      'renders attachments and requests data for $label',
+      ({ mount, hook }: { mount: () => void; hook: () => unknown }) => {
+        mount();
+
+        expect(screen.queryByText(DISABLED_MESSAGE)).toBeNull();
+        expect(screen.queryByTestId('attachment-list')).not.toBeNull();
+        expect(hook).toHaveBeenCalled();
+      },
+    );
+  });
+});

+ 60 - 0
packages/remark-attachment-refs/src/client/services/renderer/refs.spec.ts

@@ -0,0 +1,60 @@
+import { rehypePlugin } from './refs';
+
+type RefTagName = 'ref' | 'refimg' | 'refs' | 'refsimg' | 'gallery';
+
+const createTree = (
+  tagName: RefTagName,
+  properties: Record<string, unknown> = {},
+) => ({
+  type: 'root',
+  children: [
+    {
+      type: 'element',
+      tagName,
+      properties,
+      children: [],
+    },
+  ],
+});
+
+const runRehype = (
+  tree: ReturnType<typeof createTree>,
+  options: { pagePath?: string; isSharedPage?: boolean },
+): void => {
+  // WHY: unified's `Plugin` type is not directly callable; narrow it to the
+  // (options) => (tree) => void shape this factory plugin actually has.
+  (rehypePlugin as (opts: typeof options) => (tree: unknown) => void)(options)(
+    tree,
+  );
+};
+
+const getProperties = (
+  tree: ReturnType<typeof createTree>,
+): Record<string, unknown> =>
+  (tree.children[0] as { properties: Record<string, unknown> }).properties;
+
+const tags: RefTagName[] = ['ref', 'refimg', 'refs', 'refsimg', 'gallery'];
+
+describe('refs rehypePlugin - isSharedPage injection', () => {
+  it.each(
+    tags,
+  )('injects isSharedPage=true into <%s> when the option is set', (tagName) => {
+    const tree = createTree(tagName);
+    runRehype(tree, { pagePath: '/foo', isSharedPage: true });
+    expect(getProperties(tree).isSharedPage).toBe(true);
+  });
+
+  it.each(
+    tags,
+  )('leaves isSharedPage undefined on <%s> when the option is omitted', (tagName) => {
+    const tree = createTree(tagName);
+    runRehype(tree, { pagePath: '/foo' });
+    expect(getProperties(tree).isSharedPage).toBeUndefined();
+  });
+
+  it('does not override an isSharedPage already present on the element', () => {
+    const tree = createTree('refs', { isSharedPage: false });
+    runRehype(tree, { pagePath: '/foo', isSharedPage: true });
+    expect(getProperties(tree).isSharedPage).toBe(false);
+  });
+});

+ 19 - 6
packages/remark-attachment-refs/src/client/services/renderer/refs.ts

@@ -150,6 +150,7 @@ const resolvePath = (pagePath: string, basePath: string) => {
 
 type RefRehypePluginParams = {
   pagePath?: string;
+  isSharedPage?: boolean;
 };
 
 export const rehypePlugin: Plugin<[RefRehypePluginParams]> = (options = {}) => {
@@ -170,7 +171,15 @@ export const rehypePlugin: Plugin<[RefRehypePluginParams]> = (options = {}) => {
 
     for (const refElem of elements) {
       if (refElem.properties == null) {
-        continue;
+        // Initialize properties if null to avoid errors down the line
+        refElem.properties = {};
+      }
+
+      // Inject isSharedPage from the renderer config unless the element already
+      // declares it, so the directive can disable itself on a share link page.
+      const isSharedPage = refElem.properties.isSharedPage;
+      if (isSharedPage == null || typeof isSharedPage !== 'boolean') {
+        refElem.properties.isSharedPage = options.isSharedPage;
       }
 
       const prefix = refElem.properties.prefix;
@@ -199,13 +208,17 @@ export const rehypePlugin: Plugin<[RefRehypePluginParams]> = (options = {}) => {
   };
 };
 
+// Attributes injected by the renderer (not authored in markdown) that must
+// survive sanitization for every ref directive.
+const COMMON_ATTRIBUTES = ['isSharedPage'];
+
 export const sanitizeOption: SanitizeOption = {
   tagNames: ['ref', 'refimg', 'refs', 'refsimg', 'gallery'],
   attributes: {
-    ref: REF_SUPPORTED_ATTRIBUTES,
-    refimg: REF_IMG_SUPPORTED_ATTRIBUTES,
-    refs: REFS_SUPPORTED_ATTRIBUTES,
-    refsimg: REFS_IMG_SUPPORTED_ATTRIBUTES,
-    gallery: REFS_IMG_SUPPORTED_ATTRIBUTES,
+    ref: [...REF_SUPPORTED_ATTRIBUTES, ...COMMON_ATTRIBUTES],
+    refimg: [...REF_IMG_SUPPORTED_ATTRIBUTES, ...COMMON_ATTRIBUTES],
+    refs: [...REFS_SUPPORTED_ATTRIBUTES, ...COMMON_ATTRIBUTES],
+    refsimg: [...REFS_IMG_SUPPORTED_ATTRIBUTES, ...COMMON_ATTRIBUTES],
+    gallery: [...REFS_IMG_SUPPORTED_ATTRIBUTES, ...COMMON_ATTRIBUTES],
   },
 };

+ 2 - 0
packages/remark-attachment-refs/vitest.config.ts

@@ -11,6 +11,8 @@ export default defineConfig({
       // Use jsdom for client-side tests
       ['**/client/**/*.spec.ts', 'jsdom'],
       ['**/client/**/*.test.ts', 'jsdom'],
+      ['**/client/**/*.spec.tsx', 'jsdom'],
+      ['**/client/**/*.test.tsx', 'jsdom'],
     ],
   },
 });