Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @aahreum, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 메시지 카드에서 텍스트 에디터로 작성된 내용의 줄바꿈 및 서식이 사라지는 버그를 해결합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
🎉 구현한 기능 Preview: https://frontend-jeadxtbyj-fe-19-seongihuns-projects.vercel.app |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 메시지 카드에서 텍스트 줄바꿈이 사라지는 버그를 수정하고, 텍스트 에디터에서 적용된 스타일이 메시지 카드에 표시되도록 개선하는 내용을 담고 있습니다. dompurify 설정을 변경하여 더 많은 HTML 태그와 속성을 허용하도록 하셨네요. 버그 수정과 기능 개선 방향은 좋지만, style 속성을 허용하면서 발생할 수 있는 보안 위험에 대한 검토가 필요해 보입니다. 이에 대한 의견을 리뷰 코멘트로 남겼습니다.
| ALLOWED_TAGS: ['p', 'br', 'ol', 'ul', 'li', 'span', 'img'], | ||
| ALLOWED_ATTR: ['src', 'alt', 'class', 'style'], |
There was a problem hiding this comment.
메시지 카드에서 줄바꿈과 텍스트 스타일이 유지되도록 ALLOWED_TAGS와 ALLOWED_ATTR을 수정한 것은 좋은 접근입니다.
하지만 style 속성을 허용하는 것은 잠재적인 XSS(Cross-Site Scripting) 공격의 위험을 높일 수 있습니다. DOMPurify가 대부분의 위험한 CSS를 제거해주지만, 보안을 위해서는 허용하는 범위를 최소한으로 유지하는 것이 가장 좋습니다. 예를 들어, style 속성을 통해 UI를 덮어쓰거나 사용자를 속이는 등의 공격이 가능할 수 있습니다.
Quill 에디터에서 사용하는 color, background-color, text-align 등의 특정 스타일만 필요한 경우, 해당 속성들만 명시적으로 허용하는 것이 더 안전합니다. DOMPurify는 이 기능을 직접 지원하지 않지만, addHook을 사용하여 style 속성을 직접 파싱하고 안전하다고 판단되는 CSS 속성만 남기는 방식으로 구현할 수 있습니다.
이 변경은 보안에 영향을 줄 수 있으므로, 허용되는 스타일을 최소화하는 방향으로 재고해보시는 것을 권장합니다.
|
/gemini review |
|
🎉 구현한 기능 Preview: https://frontend-6e5hkgj2d-fe-19-seongihuns-projects.vercel.app |
There was a problem hiding this comment.
Code Review
이 PR은 메시지 카드에서 줄 바꿈 및 기타 서식이 사라지는 문제를 해결하기 위해 DOMPurify의 살균 설정을 업데이트합니다. 텍스트 에디터에서 적용된 스타일이 메시지 카드에 올바르게 표시되도록 허용되는 태그와 속성을 확장하고, style 속성에 허용되는 CSS 속성을 필터링하기 위한 DOMPurify 훅을 추가했습니다.
전반적으로 변경 사항은 올바르지만, 한 가지 버그와 한 가지 개선점을 발견했습니다. 자세한 내용은 개별 코드 리뷰 댓글을 확인해주세요.
| .map(([k, v]) => `${k}: ${v}`) | ||
| .join('; '); | ||
| if (!data.attrValue) { | ||
| delete node.style; |
There was a problem hiding this comment.
style 속성을 제거하기 위해 delete node.style을 사용하는 것은 올바르지 않으며, 예상대로 동작하지 않을 수 있습니다. node.style은 CSSStyleDeclaration 객체이며, delete 연산자로 DOM 요소의 속성을 제거할 수 없습니다.
속성을 제거하려면 node.removeAttribute('style')을 사용해야 합니다. 이렇게 수정하면 허용되지 않는 스타일만 포함된 경우 style 속성이 완전히 제거됩니다.
| delete node.style; | |
| node.removeAttribute('style'); |
| DOMPurify.addHook('uponSanitizeAttribute', (node, data) => { | ||
| if (data.attrName === 'style') { | ||
| const allowedStyles = [ | ||
| 'color', | ||
| 'background-color', | ||
| 'text-align', | ||
| 'font-weight', | ||
| 'font-style', | ||
| 'text-decoration', | ||
| ]; | ||
| const styleObj = {}; | ||
| data.attrValue.split(';').forEach((item) => { | ||
| const [key, value] = item.split(':').map((s) => s?.trim()); | ||
| if (key && value && allowedStyles.includes(key.toLowerCase())) { | ||
| styleObj[key] = value; | ||
| } | ||
| }); | ||
| data.attrValue = Object.entries(styleObj) | ||
| .map(([k, v]) => `${k}: ${v}`) | ||
| .join('; '); | ||
| if (!data.attrValue) { | ||
| delete node.style; | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
DOMPurify.addHook을 사용하여 훅을 등록하면 이 훅은 전역적으로 적용됩니다. 즉, 이 모듈(sanitizeConfig.js)이 임포트되는 순간부터 애플리케이션의 모든 DOMPurify.sanitize() 호출에 영향을 미치게 됩니다. 이는 DOMPurify 라이브러리의 설계이긴 하지만, 의도치 않은 부작용을 일으킬 수 있습니다.
예를 들어, 다른 곳에서 style 속성을 다른 방식으로 처리해야 하는 경우 이 전역 훅이 문제를 일으킬 수 있습니다.
이러한 전역 설정은 애플리케이션의 진입점(entry point)이나 초기화 파일과 같은 한 곳에서 명시적으로 관리하는 것을 고려해볼 수 있습니다. 만약 현재 구조를 유지한다면, 이 파일이 DOMPurify의 전역 동작을 수정한다는 것을 명확히 알리는 주석을 추가하는 것이 좋습니다.
예시 주석:
// DOMPurify의 전역 훅을 설정합니다.
// 이 훅은 애플리케이션 전체의 'style' 속성 살균에 영향을 줍니다.
DOMPurify.addHook('uponSanitizeAttribute', (node, data) => {
// ...
|
🎉 구현한 기능 Preview: https://frontend-bw332g3gu-fe-19-seongihuns-projects.vercel.app |
2 participants
🔗 이슈 번호
✨ 작업한 내용
💁 Review Point