Skip to content

security: harden response headers and dialog accessibility - #576

Open
seonghobae wants to merge 92 commits into
developfrom
fix-security-headers-and-strix-13110299334434824046
Open

security: harden response headers and dialog accessibility#576
seonghobae wants to merge 92 commits into
developfrom
fix-security-headers-and-strix-13110299334434824046

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Security and buyer-visible outcome

This branch hardens the ScopeWeave web surface without weakening authorization or deterministic gates:

  • owns the pinned response-header policy at the production runtimeApp boundary;
  • defaults HSTS to the exact ScopeWeave host and requires explicit deployment-domain proof before extending HSTS to descendants;
  • ships dialog-accessibility.js across Node, GitHub Pages, static nginx Docker, and SaaS server Docker surfaces;
  • replaces static cloud/team modal innerHTML templates with explicit DOM construction and preserves reliable close-button hit targets;
  • keeps unnamed dynamically rebuilt dialogs named across reopen by refreshing only ScopeWeave-generated aria-labelledby references while preserving explicit component labels; and
  • locks response-header and dialog-accessibility behavior into API, unit, and real-browser regressions.

Exact current identity

  • protected live base: develop@2c328875e00e86537df3e965170be80532571cad;
  • exact contributor head: 62484c2dbeb755fd80fbc123d34e55df7755bda8;
  • head tree: b916d4a81841c76733696465a68095e6e0adc9db;
  • branch: fix-security-headers-and-strix-13110299334434824046;
  • Ready and mechanically mergeable at the latest fresh refetch; and
  • no qualifying independent current-head/last-push approval is present.

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-labelledby after their panel and heading were rebuilt, leaving the reference dangling on the second and later opens. Commit 276b5e5c2541a9bfd4dfc39c93af40b4ffd20788 added the realistic RED regression. Server Tests run 32618593861, unit-and-api job 97143177681, failed at the intended assertion because the old generated id remained.

Production repair 0f6408091de90b5aa443732ee8fe9757a2b940fa now distinguishes application-owned labels from ScopeWeave-generated labels: explicit aria-label and explicit non-generated aria-labelledby remain untouched, while a generated reference is refreshed to the current visible heading when a rebuilt dialog subtree changes. On exact current head, Server Tests run 32618951414 is terminal success; unit-and-api job 97144050669 logs dialog accessibility unit tests passed, and cloud-e2e is also terminal success.

Deployment-scoped HSTS

The runtime policy emits Strict-Transport-Security: max-age=15552000 by default. includeSubDomains is emitted only when SCOPEWEAVE_HSTS_INCLUDE_SUBDOMAINS=1 is 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.md now 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 62484c2dbeb755fd80fbc123d34e55df7755bda8 are terminal success:

  • Dependency Review 32618951364;
  • Fuzz 32618951361;
  • OSV Scanner 32618951665;
  • SAST Semgrep 32618951384;
  • Server Tests 32618951414; and
  • Security Scan 32618951411.

Within Server Tests, unit-and-api job 97144050669 and cloud-e2e are terminal success. The unit log specifically records dialog accessibility unit tests passed and security 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 leaving HEAD at the immutable contributor SHA. The central reusable SAST/Security exact-head defect remains owned by ContextualWisdomLab/.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.

- 테스트 통과를 위해 `index.html`에 누락된 `modulepreload` 태그를 추가
- Hono의 `secureHeaders` 미들웨어를 사용하여 백엔드 보안 헤더(`X-Content-Type-Options`, `X-Frame-Options` 등) 일괄 적용
- 보안 스캐너 Strix의 Stored XSS 오탐을 방지하기 위해 `cloud-sync.js` 내의 `innerHTML` 사용을 `document.createElement()`로 완전 교체
@google-labs-jules

Copy link
Copy Markdown

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Hono 보안 헤더를 전체 런타임 경로에 적용했습니다. 인증 및 팀 모달을 createElement 기반으로 구성했습니다. 닫기 글리프와 대화상자 접근성 동작을 테스트로 검증합니다.

Changes

API 보안 헤더

Layer / File(s) Summary
보안 헤더 및 런타임 정적 모듈 제공
.jules/sentinel.md, server/app.mjs, server/runtime-app.mjs, server/server.mjs, Dockerfile.server, tests/api/security-headers.test.mjs
전체 요청에 secureHeaders()를 적용했습니다. dialog-accessibility.js를 제공하고 파일 오류를 404 또는 500으로 처리합니다. API, 정적 파일, 오류 응답의 헤더를 검증합니다.
런타임 모듈 테스트 계약
package.json, tests/unit/coverage-script-contract.test.mjs
런타임 라우팅 모듈을 테스트 실행 목록과 커버리지 계측 대상에 포함합니다.

클라우드 모달 DOM 하드닝

Layer / File(s) Summary
모달 DOM 생성 및 버튼 스타일
cloud-sync.js, styles.css, tests/unit/cloud-sync-security.test.mjs
인증 및 팀 모달을 개별 DOM 요소로 조립합니다. SSO 버튼의 크기와 여백을 .cloud-sso-button CSS 규칙으로 관리합니다.
닫기 글리프 이벤트 검증
toast-state.css, tests/e2e/cloud-modal-close-target.spec.js, package.json
장식용 글리프에 pointer-events: none을 적용합니다. 인증 및 팀 모달의 글리프 좌표 클릭 후 숨김 상태를 E2E 테스트로 검증합니다.

대화상자 접근성

Layer / File(s) Summary
대화상자 자동 라벨링
dialog-accessibility.js, index.html, Dockerfile, .github/workflows/pages.yml, tests/unit/dialog-accessibility.test.mjs
이름 없는 role="dialog" 요소에 표시 가능한 제목을 aria-label로 설정합니다. 초기 문서와 동적으로 추가된 대화상자를 처리하며 기존 ARIA 이름은 유지합니다. 브라우저 로드와 배포 산출물 포함 여부를 검증합니다.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: 🟡 Moderate · up to d5cee

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 설정
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 10 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 응답 보안 헤더 강화와 대화상자 접근성 개선이라는 변경의 주요 내용을 정확하고 간결하게 요약합니다.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-security-headers-and-strix-13110299334434824046

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

- 테스트 통과를 위해 `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 등 내부 리소스 대역으로의 외부 콜아웃 방지 로직 추가
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

- 테스트 통과를 위해 `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 등 내부 리소스 대역으로의 외부 콜아웃 방지 로직 추가
devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 4 new potential issues.

Open in Devin Review

Comment thread dialog-accessibility.js Outdated
Comment thread toast-state.css
Comment thread dialog-accessibility.js
Comment thread server/runtime-app.mjs

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment thread server/runtime-app.mjs
coderabbitai[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Open in Devin Review

Comment thread dialog-accessibility.js
Comment thread toast-state.css
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: accessibility Accessibility and assistive-technology support area: security Security boundary, hardening, or vulnerability prevention priority: high High-priority or P1 work status: needs-review Open pull request requiring current-head review or checks type: security Security vulnerability or security-specific remediation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant