Skip to content

Conversation

@lvalentine6
Copy link
Member

@lvalentine6 lvalentine6 commented Sep 22, 2025

✨ 개요

  • ua가 node인 경우 WAF에서 임시적으로 접속을 허용하게 수정하였습니다.
  • 로그인이 정상적으로 진행되는것을 확인하였습니다.
  • 클라이언트에서 커스텀 ua를 설정하면 해당 커스텀 ua를 가진 요청만 허용하도록 수정할 계획입니다.

🧾 관련 이슈

#202

🔍 참고 사항 (선택)

Summary by CodeRabbit

  • New Features
    • WAF가 User-Agent에 “node”가 포함되지 않은 요청을 차단하도록 규칙을 추가하여 비정상 트래픽을 더 엄격히 차단합니다.
  • Chores
    • Bot Control의 비브라우저 User-Agent 신호를 차단 대신 카운트로 집계해 트래픽 가시성과 모니터링을 개선했습니다.

@coderabbitai
Copy link

coderabbitai bot commented Sep 22, 2025

Walkthrough

AWS WAF 구성을 수정했다. 새로운 사용자 에이전트 차단 규칙을 추가했고, Bot Control 관리형 규칙셋의 특정 시그널 동작을 카운트로 오버라이드했다. 주석을 추가했다.

Changes

Cohort / File(s) Summary of changes
New WAF rule: Block-Non-Node-User-Agents
terraform/common/waf/main.tf
우선순위 41 규칙 추가: non_browser_user_agent 라벨 매치 AND NOT(User-Agent에 "node" 포함) 조건일 때 차단. 개별 visibility_config(metric_name: "block-non-node-uas") 포함.
Managed Bot Control override
terraform/common/waf/main.tf
AWSManagedRulesBotControlRuleSetrule_action_override 추가: SignalNonBrowserUserAgentcount 동작으로 설정.
Comment update
terraform/common/waf/main.tf
비-Node 사용자 에이전트 임시 패스 조건에 대한 한국어 설명 주석 추가.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant C as Client
    participant W as AWS WAF WebACL
    participant B as Managed Bot Control RuleSet
    participant R as Rule: Block-Non-Node-User-Agents
    participant O as Origin

    C->>W: HTTP Request (headers incl. User-Agent)
    W->>B: Evaluate managed bot-control rules
    note over B: SignalNonBrowserUserAgent => action: count
    B-->>W: Labels (e.g., non_browser_user_agent)

    W->>R: Evaluate composite statement
    alt Label non_browser_user_agent AND User-Agent NOT contains "node"
        R-->>W: action: block
        W-->>C: Blocked (WAF response)
    else Otherwise
        R-->>W: no match
        W-->>O: Forward request
        O-->>C: Response
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

토끼는 귀를 쫑긋, 헤더를 살핀다
"node"라 속삭이면, 길을 터준다
비브라우저 발자국, 지금은 세어본다
차단과 카운트, 규칙은 춤춘다
와프 숲 속에, 깨끗한 길이 난다 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed PR 제목 "fix: ua가 node인 경우 임시적으로 접속 허용"은 변경의 핵심(UA가 'node'인 경우를 임시 허용)을 명확하게 표현합니다. 간결하며 변경된 WAF 규칙과 일치하는 구체적인 의도를 전달합니다. 불필요한 노이즈(파일 목록, 이모지 등)가 없어 팀원이 히스토리를 스캔할 때 이해하기 쉽습니다.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hotfix/PRODUCT-284

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 and usage tips.

@github-actions
Copy link

📄 Terraform Plan Summary

🛡️ Common Infrastructure


No plan summary

Status: ✅ No Changes


🛠️ Development Environment


No plan summary

Status: ✅ No Changes


📋 Full Results: View in Actions

@lvalentine6 lvalentine6 changed the title fix: ua가 node인 경우 임시적으로 접속 허용 [Hotfix] ua가 node인 경우 임시적으로 접속 허용 Sep 22, 2025
@lvalentine6 lvalentine6 changed the title [Hotfix] ua가 node인 경우 임시적으로 접속 허용 [Hotfix] WAF 차단으로 인한 로그인 실패 문제 임시 조치 Sep 22, 2025
@sonarqubecloud
Copy link

@lvalentine6 lvalentine6 merged commit b0d300f into develop Sep 22, 2025
10 of 11 checks passed
@lvalentine6 lvalentine6 deleted the hotfix/PRODUCT-284 branch September 22, 2025 15:16
@github-actions
Copy link

🎉 This PR is included in version 1.8.0-develop.15 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
terraform/common/waf/main.tf (1)

159-164: 가시성/알림 보강

metric_name = "block-non-node-uas"에 대해 CloudWatch 경보를 추가해 급증 시 알림을 받도록 해주세요(예: 5분 합계가 평시 기준선 대비 N배 초과).

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 813a4be and 31eb94c.

📒 Files selected for processing (1)
  • terraform/common/waf/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 (2)
terraform/common/waf/main.tf (2)

129-157: 규칙 범위 의도 재확인 권장 (Non‑Browser 라벨에만 적용됨)

현재 AND 조건에 non_browser_user_agent 라벨 매치가 포함되어 있어, “ua가 node가 아니면 차단” 로직은 비(非)브라우저로 식별된 트래픽에만 적용됩니다. 일반 브라우저 UA는 이 규칙에 매치되지 않아 계속 통과합니다. 의도가 “Node UA만 통과”인지, “비브라우저 중 Node만 통과(브라우저는 영향 없음)”인지 확인해 주세요.


103-108: 확인 완료 — 서브룰 이름 및 카운트 오버라이드 라벨 유지 확인됨

terraform/common/waf/main.tf (lines 103-108)의 rule_action_override.name = "SignalNonBrowserUserAgent"은 AWS 문서상 정확하며, 해당 서브룰을 count로 오버라이드해도 라벨 'awswaf:managed:aws:bot-control:signal:non_browser_user_agent'가 계속 부착됩니다.

Comment on lines +119 to +127
# 임시 조치로 ua가 node일 경우만 통과시킴
rule {
name = "Block-Non-Node-User-Agents"
priority = 41

action {
block {}
}

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

User-Agent 기반 우회는 쉽게 스푸핑 가능 — 추가 방어선 필요

임시 조치라 하더라도 UA 문자열은 누구나 위조할 수 있습니다. 최소한 다음 중 하나 이상을 병행하세요:

  • 특정 소스 IP/VPC/LB 오리진만 허용(allowlist).
  • 전용 경로/리소스(예: 로그인)로 범위 축소.
  • 커스텀 시크릿 헤더/HMAC 서명 또는 mTLS로 식별 강화.

향후 커스텀 UA로 전환 시, UA만으로 신뢰하지 말고 상기 수단 중 하나를 함께 적용해 주세요.

🤖 Prompt for AI Agents
In terraform/common/waf/main.tf around lines 119-127, the WAF rule currently
allowing requests based solely on User-Agent ("node") is insecure because UA can
be spoofed; replace or augment this UA-only rule with at least one stronger
control: restrict by source (allowlist specific IP ranges, VPC or LB origin),
narrow the rule to specific paths/resources (e.g., only login endpoints), or
require a strong provenance token such as a custom secret header with HMAC
signature or mTLS. Update the WAF rule to include the chosen additional
condition(s) and corresponding configuration (trusted IPs, path matching,
header/HMAC validation, or client cert requirement) so UA-based allowance is not
the sole trust signal.

Comment on lines +141 to +151
search_string = "node"
field_to_match {
single_header {
name = "user-agent"
}
}
positional_constraint = "CONTAINS"
text_transformation {
priority = 0
type = "NONE"
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

케이스 민감도 때문에 실제 Node UA가 차단될 수 있음 (치명적)

search_string = "node"인데 text_transformationNONE라서 User-Agent: Node.js/18...처럼 대문자/혼합 케이스는 매치되지 않습니다. 현재 규칙은 "NOT contains 'node'" 조건을 만족시키므로 오히려 Node 클라이언트를 차단할 수 있습니다. 반드시 소문자 정규화를 적용하세요.

아래 패치를 적용하세요:

-                text_transformation {
-                  priority = 0
-                  type     = "NONE"
-                }
+                text_transformation {
+                  priority = 0
+                  type     = "LOWERCASE"
+                }

또는 regex_pattern_set_reference_statement(?i)node(대소문자 무시) 사용을 권장합니다.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
search_string = "node"
field_to_match {
single_header {
name = "user-agent"
}
}
positional_constraint = "CONTAINS"
text_transformation {
priority = 0
type = "NONE"
}
search_string = "node"
field_to_match {
single_header {
name = "user-agent"
}
}
positional_constraint = "CONTAINS"
text_transformation {
priority = 0
type = "LOWERCASE"
}
🤖 Prompt for AI Agents
In terraform/common/waf/main.tf around lines 141 to 151, the rule uses
search_string = "node" with text_transformation = "NONE", causing mixed-case
User-Agent values like "Node.js/18" to miss the match; update the statement to
either (A) add a text_transformation block that lowercases the inspected text
(type = "LOWERCASE", priority appropriately set) so the CONTAINS check matches
any case variant, or (B) replace the current string statement with a
regex_pattern_set_reference_statement using a case-insensitive pattern like
(?i)node and reference that regex set; implement one of these fixes and ensure
priority/order of transformations is correct.

@github-actions
Copy link

github-actions bot commented Oct 1, 2025

🎉 This PR is included in version 1.9.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants