security: harden response headers and dialog accessibility - #576
security: harden response headers and dialog accessibility#576seonghobae wants to merge 92 commits into
Conversation
- 테스트 통과를 위해 `index.html`에 누락된 `modulepreload` 태그를 추가 - Hono의 `secureHeaders` 미들웨어를 사용하여 백엔드 보안 헤더(`X-Content-Type-Options`, `X-Frame-Options` 등) 일괄 적용 - 보안 스캐너 Strix의 Stored XSS 오탐을 방지하기 위해 `cloud-sync.js` 내의 `innerHTML` 사용을 `document.createElement()`로 완전 교체
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHono 보안 헤더를 전체 런타임 경로에 적용했습니다. 인증 및 팀 모달을 ChangesAPI 보안 헤더
클라우드 모달 DOM 하드닝
대화상자 접근성
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟡 Moderate · up to Dynamically updated dialogs may remain without an accessible name when heading text is inserted as a text node, creating a screen-reader accessibility regression; this should be fixed with regression coverage before merge. Required exact-head checks and qualifying approval are also not yet complete. Sequence Diagram(s)sequenceDiagram
participant Document
participant MutationObserver
participant labelUnnamedDialogs
Document->>labelUnnamedDialogs: 초기 대화상자와 하위 대화상자 전달
labelUnnamedDialogs-->>Document: 제목 기반 aria-label 설정
MutationObserver->>labelUnnamedDialogs: addedNodes 하위 트리 전달
labelUnnamedDialogs-->>Document: 동적 대화상자 aria-label 설정
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- 테스트 통과를 위해 `index.html`에 누락된 `modulepreload` 태그를 추가 - Hono의 `secureHeaders` 미들웨어를 사용하여 백엔드 보안 헤더(`X-Content-Type-Options`, `X-Frame-Options` 등) 일괄 적용 - 보안 스캐너 Strix의 Stored XSS 오탐을 방지하기 위해 `cloud-sync.js` 내의 `innerHTML` 사용을 `document.createElement()`로 완전 교체 - `team-modal` 생성 시 접근성 속성을 올바르게 적용하도록 수정 - Strix 검사 실패의 원인이었던 GitHub Actions 워크플로 스크립트(`fuzz.yml`)의 셸 인젝션 취약점을 안전한 환경변수 참조 방식으로 수정 - Webhook endpoints에 존재하는 IDOR(Insecure Direct Object Reference) 취약점 수정 (리소스 소유권 검증 로직 강화) - SSRF(Server-Side Request Forgery) 방어 강화: `/api/orgs/:id/webhooks`에서 Loopback/Private IP/Cloud metadata endpoint 등 내부 리소스 대역으로의 외부 콜아웃 방지 로직 추가
- 테스트 통과를 위해 `index.html`에 누락된 `modulepreload` 태그를 추가 - Hono의 `secureHeaders` 미들웨어를 사용하여 백엔드 보안 헤더(`X-Content-Type-Options`, `X-Frame-Options` 등) 일괄 적용 - 보안 스캐너 Strix의 Stored XSS 오탐을 방지하기 위해 `cloud-sync.js` 내의 `innerHTML` 사용을 `document.createElement()`로 완전 교체 - `team-modal` 생성 시 접근성 속성을 올바르게 적용하도록 수정 - Strix 검사 실패의 원인이었던 GitHub Actions 워크플로 스크립트(`fuzz.yml`)의 셸 인젝션 취약점을 안전한 환경변수 참조 방식으로 수정 - Webhook endpoints에 존재하는 IDOR(Insecure Direct Object Reference) 취약점 수정 (리소스 소유권 검증 로직 강화) - SSRF(Server-Side Request Forgery) 방어 강화: `/api/orgs/:id/webhooks`에서 Loopback/Private IP/Cloud metadata endpoint 등 내부 리소스 대역으로의 외부 콜아웃 방지 로직 추가
Security and buyer-visible outcome
This branch hardens the ScopeWeave web surface without weakening authorization or deterministic gates:
runtimeAppboundary;dialog-accessibility.jsacross Node, GitHub Pages, static nginx Docker, and SaaS server Docker surfaces;innerHTMLtemplates with explicit DOM construction and preserves reliable close-button hit targets;aria-labelledbyreferences while preserving explicit component labels; andExact current identity
develop@2c328875e00e86537df3e965170be80532571cad;62484c2dbeb755fd80fbc123d34e55df7755bda8;b916d4a81841c76733696465a68095e6e0adc9db;fix-security-headers-and-strix-13110299334434824046;Any contributor-head or protected-base movement invalidates head/base-sensitive evidence until the exact revisions, review state, checkout attestations, checks, and live-base identity are freshly revalidated.
Review-driven repairs
The branch retains regression-backed fixes for the runtime static-module route, injectable/delegated runtime app construction, deployment-surface inclusion, dialog naming, modal close-target behavior, stylesheet ownership, mutation-observer behavior, and fuzz-budget input validation. It also removed accidental merge/patch artifacts and an obsolete Trivy suppression rather than masking the patched dependency advisory.
A shallow webhook-hostname blocklist and its non-deterministic smoke-test workaround were removed from this lane. Comprehensive outbound webhook SSRF ownership remains with #552/#551, whose DNS-aware design is the correct causal boundary; this PR does not duplicate or weaken that work.
Reopen-safe dialog naming
Current-head review identified a real reopen defect: persistent unnamed dialogs retained a generated
aria-labelledbyafter their panel and heading were rebuilt, leaving the reference dangling on the second and later opens. Commit276b5e5c2541a9bfd4dfc39c93af40b4ffd20788added the realistic RED regression. Server Tests run32618593861,unit-and-apijob97143177681, failed at the intended assertion because the old generated id remained.Production repair
0f6408091de90b5aa443732ee8fe9757a2b940fanow distinguishes application-owned labels from ScopeWeave-generated labels: explicitaria-labeland explicit non-generatedaria-labelledbyremain untouched, while a generated reference is refreshed to the current visible heading when a rebuilt dialog subtree changes. On exact current head, Server Tests run32618951414is terminal success;unit-and-apijob97144050669logsdialog accessibility unit tests passed, andcloud-e2eis also terminal success.Deployment-scoped HSTS
The runtime policy emits
Strict-Transport-Security: max-age=15552000by default.includeSubDomainsis emitted only whenSCOPEWEAVE_HSTS_INCLUDE_SUBDOMAINS=1is set after deployment inventory proves all current and future descendant hosts are HTTPS-capable and under certificate/routing ownership. Shared apex and customer-managed namespaces remain excluded without that proof.docs/doctoring/hsts-deployment-scope.mdnow records this as an accepted durable deployment decision rather than PR-relative status text.Current review state
All verified current defect threads addressed by this branch are resolved. Remaining non-outdated unresolved threads are informational analysis only: close-glyph pointer-event behavior, bounded global observer cost, runtime header compatibility, and CSP ownership. They do not identify a current correctness or security defect.
Model-only/comment-only review evidence is not counted as independent approval. No qualifying independent current-head/last-push approval exists.
Current CI evidence
All six repository-native workflow runs associated with contributor head
62484c2dbeb755fd80fbc123d34e55df7755bda8are terminal success:32618951364;32618951361;32618951665;32618951384;32618951414; and32618951411.Within Server Tests,
unit-and-apijob97144050669andcloud-e2eare terminal success. The unit log specifically recordsdialog accessibility unit tests passedandsecurity header regression passed.These successes are product-behavior evidence, not exact-contributor-head merge authority. The Server Tests checkout fetched and checked out synthetic merge commit
51e8ae6bbe339613ac0da92a55e2739b9773f2f8(Merge 62484c2dbeb755fd80fbc123d34e55df7755bda8 into 2c328875e00e86537df3e965170be80532571cad) rather than leavingHEADat the immutable contributor SHA. The central reusable SAST/Security exact-head defect remains owned byContextualWisdomLab/.github#1222; the repository-native Server Tests/coverage exact-head control lane remains #523. Neither synthetic-merge success nor predecessor success substitutes for exact-current-contributor-head evidence.This PR therefore does not claim exact 100% owned production coverage on the current branch. Coverage and exact-head workflow evidence must be regenerated after the corresponding control repairs integrate.
Merge boundary
Do not merge or enable auto-merge until the unchanged exact contributor head remains freshly reconciled to protected
develop, the repository and central exact-head control repairs are protected-shipped, every applicable exact-head CI/browser/owned-coverage/docstring/CodeQL/SAST/security/dependency/supply-chain/package/provenance gate is substantively terminal-passing, valid unresolved findings are zero, and live governance has a genuine qualifying independent current-head/last-push approval. Pending, queued, skipped-required, cancelled, absent, neutral-required, failed, stale, predecessor, synthetic-only, status-only, author-only, model-only, rate-limited, or infrastructure-only evidence is non-passing. No certification claim is made by this PR.