fix(web): P66 follow-up — Gemini PR #75 security/a11y 리뷰 반영#77
Conversation
Gemini PR #75 리뷰의 보안 / 접근성 finding 3건 반영. heading collapse 의 DOM 직접 조작 한계 (MEDIUM finding 중 하나) 는 별도 plan 으로 분리 — AST 기반 state-driven 재설계가 필요. ## 변경 ### 1. rehype-sanitize 추가 (HIGH security) `rehype-raw` 가 raw HTML 을 통과시키면 XSS 위험. `rehype-sanitize` 를 중간 단계로 추가해 위험 태그/속성 제거. - `pnpm add rehype-sanitize` - pipeline: `rehype-raw` → `rehype-sanitize` (커스텀 schema) → `rehype-highlight` - schema: `defaultSchema` 에 `<details>` / `<summary>` 태그 + highlight.js className (`hljs-*`, `language-*`) 허용 추가. 그 외 script/style/event handler 등은 defaultSchema 가 차단. ### 2. `@ungap/structured-clone` CVE-CWE-502 (HIGH security) `hast-util-raw` 의 transitive dep `@ungap/structured-clone@1.3.0` 가 deprecated (CWE-502, Deserialization). pnpm overrides 로 1.3.1+ 강제. ```json "pnpm": { "overrides": { "@ungap/structured-clone": "^1.3.1" } } ``` ### 3. CollapsibleHeading 키보드 접근성 (MEDIUM) `role="button"` + `tabIndex={0}` 이 있어도 `onKeyDown` 핸들러 없으면 키보드 사용자가 폴딩 못 함. `Enter` / `Space` 시 `onClick` 과 동일 동작하도록 핸들러 추가. ## 미해결 (별도 plan) Gemini 의 한 finding 은 본 PR 에 포함 안 됨: - **CollapsibleHeading 의 DOM 직접 조작**: `nextElementSibling` + `style.display` 는 React 의 선언적 모델과 충돌. 마크다운 AST 분석 후 state 기반 가시성 제어로 재설계 필요 (큰 변경). web-backlog 후속. ## 검증 - `pnpm typecheck` 통과 - `pnpm build` 통과 (sanitize 추가로도 청크 크기 영향 미미) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request enhances security and accessibility in the Markdown rendering component. Key changes include the integration of rehype-sanitize with a custom schema to prevent XSS while maintaining support for syntax highlighting and collapsible elements, and a dependency override for @ungap/structured-clone to address a security vulnerability. Accessibility is improved by adding keyboard support to collapsible headings. Feedback suggests refining the onKeyDown handler to prevent blocking interactions with nested links or code blocks and updating the sanitization schema to permit the open attribute on details tags.
| const onKeyDown = (e: KeyboardEvent<HTMLElement>) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| e.preventDefault(); | ||
| onClick(e as unknown as MouseEvent<HTMLElement>); | ||
| } | ||
| }; |
There was a problem hiding this comment.
onKeyDown 핸들러에서 e.preventDefault()를 호출하기 전에 이벤트 타겟이 링크(<a>)나 코드 블록(<code>)인지 확인하는 로직이 누락되었습니다. 이로 인해 헤딩 내부의 링크를 키보드(Enter 키)로 클릭할 때 preventDefault()가 먼저 실행되어 링크 이동이 작동하지 않게 됩니다. onClick 핸들러와 동일하게 타겟을 체크하여 조기에 반환하도록 수정이 필요합니다.
const onKeyDown = (e: KeyboardEvent<HTMLElement>) => {
if (e.key === "Enter" || e.key === " ") {
const target = e.target as HTMLElement;
if (target.closest("a") || target.closest("code")) return;
e.preventDefault();
onClick(e as unknown as MouseEvent<HTMLElement>);
}
};
| attributes: { | ||
| ...(defaultSchema.attributes ?? {}), | ||
| code: [ | ||
| ...((defaultSchema.attributes ?? {}).code ?? []), | ||
| ["className", /^language-/, /^hljs/], | ||
| ], | ||
| span: [ | ||
| ...((defaultSchema.attributes ?? {}).span ?? []), | ||
| ["className", /^hljs-/], | ||
| ], | ||
| }, |
There was a problem hiding this comment.
rehype-sanitize 스키마에 details 태그의 open 속성을 허용하는 설정이 누락되었습니다. 마크다운 내에 <details open>과 같이 초기 상태가 지정된 경우나, 브라우저의 기본 동작으로 open 속성이 변경될 때 스키마에서 이를 허용하지 않으면 의도치 않게 속성이 제거될 수 있습니다.
attributes: {
...(defaultSchema.attributes ?? {}),
details: ["open"],
code: [
...((defaultSchema.attributes ?? {}).code ?? []),
["className", /^language-/, /^hljs/],
],
span: [
...((defaultSchema.attributes ?? {}).span ?? []),
["className", /^hljs-/],
],
},
## HIGH: heading 내부 link 의 키보드 클릭 무효화 `onKeyDown` 핸들러가 target check 없이 `e.preventDefault()` 호출 → heading 내부 `<a>` 를 Enter 키로 클릭 시 navigation 막힘. `onClick` 과 동일하게 target 이 a/code 면 early return. ## MEDIUM: <details open> 속성 strip sanitize schema 에 `details: ["open"]` 추가. 마크다운 안의 `<details open>` 또는 브라우저 토글 시 추가되는 open 속성이 보존되도록. ## 검증 - pnpm typecheck 통과 - pnpm build 통과 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Gemini PR #75 리뷰의 보안 / 접근성 finding 3건 반영.
rehype-rawXSS 위험rehype-sanitize중간 단계 추가@ungap/structured-clone@1.3.0CWE-502onKeyDown누락sanitize schema
defaultSchema베이스에:<details>/<summary>태그 허용 (폴딩 동작)hljs-*,language-*) 허용 (syntax highlight)Test plan
pnpm typecheckpnpm build<script>태그가 포함된 md 렌더링 시 차단되는지 확인미해결 (별도 plan)
CollapsibleHeading 의 DOM 직접 조작 (
nextElementSibling+style.display) → AST 분석 후 state 기반 재설계가 필요. web-backlog 항목으로.🤖 Generated with Claude Code