-
Notifications
You must be signed in to change notification settings - Fork 0
[Fix] ua 조건 제거 및 x-origin-verify Header 검사 추가 #204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough웹 ACL의 기본 동작을 allow→block으로 변경하고 SSM 파라미터 기반 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as 클라이언트
participant E as 엣지(CloudFront/ALB)
participant W as WAF v2
participant S as SSM Parameter Store
C->>E: HTTP 요청 (헤더 포함)
E->>W: WebACL 평가 시작
Note over W: 주요 변경사항\n- Allow-Verified-Server-Requests (UA 포함 + x-origin-verify EXACT)\n- Allow-Browser-Requests (origin/referer 존재)\n- Rate-Limit 및 Managed Rule 우선순위 변경\n- 로깅 시 `x-origin-verify` 헤더 마스킹
W->>S: (plan/apply 시) `x_origin_verify` 파라미터 참조
alt UA contains "node" or "Vercel" AND x-origin-verify == secret
W-->>E: Allow (Verified server path)
else origin or referer size > 0
W-->>E: Allow (Browser-like path)
else rate-limit or managed-rule 트리거
W-->>E: Block (403)
end
E-->>C: 응답 반환
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
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 |
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
terraform/common/waf/main.tf (2)
1-3: SSM 파라미터가 SecureString이면 복호화 옵션 추가 필요SecureString일 경우 with_decryption 미설정 시 plan/apply 실패하거나 암호문이 들어갈 수 있습니다. 또한 state에 평문 비밀값이 저장되므로(필연적) 백엔드(S3/KMS 등) 암호화·권한을 반드시 확인하세요. Terraform IAM에 ssm:GetParameter, kms:Decrypt 권한도 점검 바랍니다.
다음 수정 제안:
data "aws_ssm_parameter" "x_origin_verify" { name = "/common/CLIENT_HEADER_SECRET" + with_decryption = true }
33-64: 회전(로테이션) 가능하게 2개 토큰 동시 허용 고려무중단 비밀 회전을 위해 현재값과 차기값을 OR로 허용하도록 구성(RegexPatternSet 또는 두 개의 byte_match OR)하면 롤아웃/롤백이 수월합니다. 운영 정책에 맞으면 추후 반영을 권장합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
terraform/common/waf/main.tf(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: ✅ No Changes 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: 🔄 Changes Detected 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
terraform/common/waf/main.tf (1)
133-136: 레이트 제한 값 참조 오류 가능성 — var → local 정합성현재
limit = var.request_threshold인데, 변경된 값은locals.tf의local.request_threshold입니다. 불일치로 인해 2000이 적용되지 않을 수 있습니다. 한쪽으로 통일하세요.즉시 적용 예:
- limit = var.request_threshold + limit = local.request_threshold
🧹 Nitpick comments (1)
terraform/common/waf/main.tf (1)
79-124: 브라우저 식별 강화를 검토Origin/Referer 존재만으로 허용하면 비브라우저가 임의 헤더로 우회 가능합니다.
Sec-Fetch-Site(또는Sec-Fetch-Mode) 존재 조건을 추가하면 완화됩니다.예시(OR에 추가):
or_statement { statement { ... origin size > 0 ... } statement { ... referer size > 0 ... } + statement { + size_constraint_statement { + field_to_match { single_header { name = "sec-fetch-site" } } + comparison_operator = "GT" + size = 0 + text_transformation { priority = 0 type = "NONE" } + } + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
terraform/common/locals.tf(1 hunks)terraform/common/waf/main.tf(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
terraform/common/locals.tf (1)
253-254: Rate limit 값 변경이 WAF에 반영되지 않을 수 있음 — var vs local 불일치 확인 필요위치: terraform/common/locals.tf:253-254
local.request_threshold = 2000으로 설정했으나 WAF가 var.request_threshold를 참조하면 변경이 반영되지 않습니다. WAF가 local.request_threshold를 사용하도록 수정하거나 variables.tf의 기본값/모듈 입력을 2000으로 업데이트하세요.자동 검색이 실패했습니다. 레포 루트에서 사용처/변수 정의를 직접 확인하세요:
rg -n 'var.request_threshold|local.request_threshold' || true
rg -n 'variable\s*"request_threshold"' || true
📄 Terraform Plan Summary🛡️ Common InfrastructureStatus: 🔄 Changes Detected 🛠️ Development EnvironmentStatus: ✅ No Changes 📋 Full Results: View in Actions |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
terraform/common/main.tf (1)
77-81: 민감 헤더 추가 스크러빙 제안 (Authorization, Cookie).운영 로그에 자격증명/세션 정보가 남지 않도록 Authorization, Cookie도 함께 마스킹하는 것을 권장합니다.
아래와 같이 redacted_fields 블록을 추가해 주세요:
resource "aws_wafv2_web_acl_logging_configuration" "this" { log_destination_configs = [trimsuffix(aws_cloudwatch_log_group.waf_logs.arn, ":*")] resource_arn = module.waf.web_acl_arn redacted_fields { single_header { name = "x-origin-verify" } } + redacted_fields { + single_header { + name = "authorization" + } + } + redacted_fields { + single_header { + name = "cookie" + } + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
terraform/common/main.tf(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
terraform/common/main.tf (1)
77-81: 로그 스크러빙에 x-origin-verify 추가 — WAF 매칭 확인됨terraform/common/main.tf의 redacted_fields와 terraform/common/waf/main.tf의 single_header(byte_match_statement)에서 동일한 소문자 "x-origin-verify" 사용을 확인했습니다.
|
/noti |
leegwichan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/noti 서비스 신경써주셔서 감사함니다 LGTM~!
|
🎉 This PR is included in version 1.8.0-develop.16 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.9.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |



✨ 개요
🧾 관련 이슈
Close #202
🔍 참고 사항 (선택)
Summary by CodeRabbit
New Features
Bug Fixes
Chores