test: покрыть тестами sandbox HTML-embed и trackerHead (ревью 81823fce..HEAD) #99

Closed
opened 2026-06-21 03:39:13 +03:00 by Ghost · 0 comments

Контекст

Финальное мульти-аспектное ревью коммитов 81823fce^..HEAD (sandbox HTML-embed, admin-only trackerHead, удаление role-gating) показало: реализация корректна и безопасна, но самые чувствительные новые пути поставлены без тестов. Issue собирает пробелы покрытия для закрытия перед релизом.

Уже покрыто (для контекста, не трогаем): prepareContentForShare (toggle ON/OFF/absent/workspace-missing + транклюзия) в share-html-embed.spec.ts; персистентность trackerHead (updateSetting) в workspace-html-embed.spec.ts; чистые хелперы песочницы (buildSandboxSrcdoc / shouldRender / canEdit) в html-embed-sandbox.test.ts.

Пробелы покрытия

1. Инъекция trackerHead в <head> — нулевое покрытие (важно)

Файл: apps/server/src/core/share/share-seo.controller.ts (≈L93–115)

Самый чувствительный новый путь (verbatim-инъекция доверенного admin-сниппета в origin публичной страницы) не покрыт ни одним spec. Непокрытые ветки:

  • function-replacer — фикс, ради которого делался коммит: строковый String.replace интерпретировал бы $& / $` / $' / $$ внутри сниппета как шаблоны подстановки и портил бы трекер;
  • ветка отсутствия маркера </head> (logger.warn, без инъекции);
  • пустой / whitespace / не-строка trackerHead → инъекции нет.

Что сделать: вынести инъекцию в чистый хелпер injectTrackerHead(html, trackerHead): string и покрыть юнит-тестами:

  • сниппет с $& / $$ / $` / $' вставляется байт-в-байт перед </head> (тест падает на до-фиксовом строковом replace);
  • пустой / whitespace / не-строка → html без изменений;
  • нет </head> → html без изменений.

2. Passthrough-узел htmlEmbed в mcp-схеме без round-trip теста (важно)

Файл: packages/mcp/src/lib/docmost-schema.ts (узел HtmlEmbed)

Узел добавлен, чтобы TiptapTransformer.toYdoc не падал с Unknown node type: htmlEmbed при MCP/AI-правке страницы с embed. В packages/mcp/test — ноль упоминаний htmlEmbed, фикс не зафиксирован тестом; удаление узла из docmostExtensions молча вернёт падение.

Что сделать: добавить htmlEmbed в объект cases в packages/mcp/test/unit/schema.test.mjs (там уже есть acceptance-цикл по toYdoc); кейс round-trip, проверяющий сохранение source / height.

3. Клиентский авто-resize / валидация источника / clampHeight / sandbox-атрибут (важно)

Файл: apps/client/src/features/editor/components/html-embed/html-embed-view.tsx (≈L54–95, L156)

html-embed-sandbox.test.ts п��крывает только чистые хелперы. Непокрыто:

  • валидация источника postMessage event.source !== iframeRef.current?.contentWindow (единственная защита от чужого окна);
  • фильтр типа сообщения; Number.isFinite-гард (NaN→игнор); clampHeight [40, 4000];
  • атрибут sandbox="allow-scripts allow-popups allow-forms" (несущее свойство всей защиты от XSS) — ничем не застрахован от регрессии.

Что сделать: вынести предикаты (isTrustedHeightMessage, экспортировать clampHeight) в html-embed-sandbox.ts и юнит-тестировать (чужой source отвергнут, неверный тип отвергнут, NaN/Infinity → нет апдейта, clamp на обеих границах); добавить тест/ассерт, что у iframe ровно эти три sandbox-токена без allow-same-origin и задан srcDoc, а не src.

4. Атрибут height в editor-ext (parse/render + NaN-гард)

Файл: packages/editor-ext/src/lib/html-embed/html-embed.ts (≈L96–105)

Существующий html-embed-codec.spec.ts покрывает только кодек source. Проверить parseHTML: "120"→120, отсутствует→null, "abc"→null (NaN-гард, падает на до-фиксовом parseInt-only коде), "120px"→120; renderHTML: 120→{"data-height":"120"}, null/0→{}.

5. Валидация DTO trackerHead (@IsString / @MaxLength(20000))

Файл: apps/server/src/core/workspace/dto/update-workspace.dto.ts (≈L62–67)

Валидация не тестируется; новый workspace-html-embed.spec.ts обходит ValidationPipe через as any, так что предел 20000 символов и проверка типа не проверяются. В проекте нет прецедента *.dto.spec.ts, поэтому это самый нижний приоритет — но MaxLength это реальная граница на влияемый админом ввод в <head>.

(Опционально, связанное) Контрактный тест против дрейфа схемы

Схема htmlEmbed живёт в трёх копиях (editor-ext — канон, mcp — ручное зеркало, server-util). Лёгкий контрактный тест, прогоняющий фикстуру через editor-ext и mcp-зеркало и сверяющий имя узла + ключи атрибутов, превратил бы молчаливый рантайм-дрейф в падение CI. Не обязательно в рамках этого issue.

Definition of Done

  • (1) Хелпер injectTrackerHead + юнит-тесты (вкл. $-последовательности)
  • (2) Round-trip тест htmlEmbed в mcp
  • (3) Юнит-тесты обработчика resize / валидации / clamp + ассерт sandbox-атрибута
  • (4) Тесты атрибута height в editor-ext (вкл. NaN-гард)
  • (5) (опц.) Тест валидации DTO trackerHead
  • (опц.) Контрактный тест синхронизации схемы editor-ext ↔ mcp

Issue создан по результатам код-ревью (skill code-review-orchestrator), диапазон 81823fce^..HEAD.

## Контекст Финальное мульти-аспектное ревью коммитов `81823fce^..HEAD` (sandbox HTML-embed, admin-only `trackerHead`, удаление role-gating) показало: реализация корректна и безопасна, но самые чувствительные новые пути поставлены **без тестов**. Issue собирает пробелы покрытия для закрытия перед релизом. **Уже покрыто** (для контекста, не трогаем): `prepareContentForShare` (toggle ON/OFF/absent/workspace-missing + транклюзия) в `share-html-embed.spec.ts`; персистентность `trackerHead` (`updateSetting`) в `workspace-html-embed.spec.ts`; чистые хелперы песочницы (`buildSandboxSrcdoc` / `shouldRender` / `canEdit`) в `html-embed-sandbox.test.ts`. ## Пробелы покрытия ### 1. Инъекция `trackerHead` в `<head>` — нулевое покрытие (важно) **Файл:** `apps/server/src/core/share/share-seo.controller.ts` (≈L93–115) Самый чувствительный новый путь (verbatim-инъекция доверенного admin-сниппета в origin публичной страницы) не покрыт ни одним spec. Непокрытые ветки: - function-replacer — фикс, ради которого делался коммит: строковый `String.replace` интерпретировал бы `$&` / `` $` `` / `$'` / `$$` внутри сниппета как шаблоны подстановки и портил бы трекер; - ветка отсутствия маркера `</head>` (logger.warn, без инъекции); - пустой / whitespace / не-строка `trackerHead` → инъекции нет. **Что сделать:** вынести инъекцию в чистый хелпер `injectTrackerHead(html, trackerHead): string` и покрыть юнит-тестами: - сниппет с `$&` / `$$` / `` $` `` / `$'` вставляется байт-в-байт перед `</head>` (тест падает на до-фиксовом строковом replace); - пустой / whitespace / не-строка → html без изменений; - нет `</head>` → html без изменений. ### 2. Passthrough-узел `htmlEmbed` в mcp-схеме без round-trip теста (важно) **Файл:** `packages/mcp/src/lib/docmost-schema.ts` (узел `HtmlEmbed`) Узел добавлен, чтобы `TiptapTransformer.toYdoc` не падал с `Unknown node type: htmlEmbed` при MCP/AI-правке страницы с embed. В `packages/mcp/test` — ноль упоминаний `htmlEmbed`, фикс не зафиксирован тестом; удаление узла из `docmostExtensions` молча вернёт падение. **Что сделать:** добавить `htmlEmbed` в объект `cases` в `packages/mcp/test/unit/schema.test.mjs` (там уже есть acceptance-цикл по `toYdoc`); кейс round-trip, проверяющий сохранение `source` / `height`. ### 3. Клиентский авто-resize / валидация источника / clampHeight / sandbox-атрибут (важно) **Файл:** `apps/client/src/features/editor/components/html-embed/html-embed-view.tsx` (≈L54–95, L156) `html-embed-sandbox.test.ts` п��крывает только чистые хелперы. Непокрыто: - валидация источника postMessage `event.source !== iframeRef.current?.contentWindow` (единственная защита от чужого окна); - фильтр типа сообщения; `Number.isFinite`-гард (NaN→игнор); `clampHeight` [40, 4000]; - атрибут `sandbox="allow-scripts allow-popups allow-forms"` (несущее свойство всей защиты от XSS) — ничем не застрахован от регрессии. **Что сделать:** вынести предикаты (`isTrustedHeightMessage`, экспортировать `clampHeight`) в `html-embed-sandbox.ts` и юнит-тестировать (чужой source отвергнут, неверный тип отвергнут, NaN/Infinity → нет апдейта, clamp на обеих границах); добавить тест/ассерт, что у iframe ровно эти три sandbox-токена без `allow-same-origin` и задан `srcDoc`, а не `src`. ### 4. Атрибут `height` в editor-ext (parse/render + NaN-гард) **Файл:** `packages/editor-ext/src/lib/html-embed/html-embed.ts` (≈L96–105) Существующий `html-embed-codec.spec.ts` покрывает только кодек `source`. Проверить parseHTML: `"120"`→120, отсутствует→null, `"abc"`→null (NaN-гард, падает на до-фиксовом `parseInt`-only коде), `"120px"`→120; renderHTML: 120→`{"data-height":"120"}`, null/0→`{}`. ### 5. Валидация DTO `trackerHead` (`@IsString` / `@MaxLength(20000)`) **Файл:** `apps/server/src/core/workspace/dto/update-workspace.dto.ts` (≈L62–67) Валидация не тестируется; новый `workspace-html-embed.spec.ts` обходит ValidationPipe через `as any`, так что предел 20000 символов и проверка типа не проверяются. В проекте нет прецедента `*.dto.spec.ts`, поэтому это самый нижний приоритет — но `MaxLength` это реальная граница на влияемый админом ввод в `<head>`. ## (Опционально, связанное) Контрактный тест против дрейфа схемы Схема `htmlEmbed` живёт в трёх копиях (editor-ext — канон, mcp — ручное зеркало, server-util). Лёгкий контрактный тест, прогоняющий фикстуру через editor-ext и mcp-зеркало и сверяющий имя узла + ключи атрибутов, превратил бы молчаливый рантайм-дрейф в падение CI. Не обязательно в рамках этого issue. ## Definition of Done - [ ] (1) Хелпер `injectTrackerHead` + юнит-тесты (вкл. `$`-последовательности) - [ ] (2) Round-trip тест `htmlEmbed` в mcp - [ ] (3) Юнит-тесты обработчика resize / валидации / clamp + ассерт sandbox-атрибута - [ ] (4) Тесты атрибута `height` в editor-ext (вкл. NaN-гард) - [ ] (5) (опц.) Тест валидации DTO `trackerHead` - [ ] (опц.) Контрактный тест синхронизации схемы editor-ext ↔ mcp --- _Issue создан по результатам код-ревью (skill `code-review-orchestrator`), диапазон `81823fce^..HEAD`._
Ghost added the test label 2026-06-21 03:39:13 +03:00
Ghost closed this issue 2026-06-21 14:10:34 +03:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#99