Conversation
📝 WalkthroughWalkthroughThis PR adds Spring Boot Actuator and Prometheus metric exposure for application monitoring, configures Grafana Alloy integration with Grafana Cloud, and introduces comprehensive feature specifications for email-based authentication and password-reset functionality. Updates are primarily configuration, documentation, and state management with one permission-enabling change to SecurityConfig. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.omc/state/mission-state.json (1)
1-145:⚠️ Potential issue | 🟡 MinorAdd
.omc/to.gitignore.The
.omc/state/mission-state.jsonfile contains auto-generated tool state tracking mission sessions and agent timelines. This directory is not currently in.gitignoreand should be excluded from version control as it represents ephemeral development tool state unrelated to the PR's objective.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.omc/state/mission-state.json around lines 1 - 145, The repo is committing auto-generated tool state (.omc/state/mission-state.json); update the repository ignore rules by adding the .omc/ directory to .gitignore so all files under .omc (e.g., .omc/state/mission-state.json) are excluded from version control, commit the updated .gitignore, and remove any already-tracked .omc files from the index (git rm --cached) before committing to ensure the directory is ignored going forward.src/main/java/com/kospot/common/security/config/SecurityConfig.java (1)
86-99:⚠️ Potential issue | 🔴 CriticalGlobal
permitAlldisables JWT authorization across the API.Line 98 (
anyRequest().permitAll()) makes every endpoint public, so the new actuator matcher (Line 96) is effectively moot and the app is open by default. This should beanyRequest().authenticated()with only explicit public matchers permitted.As per coding guidelines "Use Spring Security with JWT bearer tokens; OAuth2 login endpoints issue JWTs; allow anonymous access for practice games, rankings, and shop browsing".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/kospot/common/security/config/SecurityConfig.java` around lines 86 - 99, In configureAuth(HttpSecurity) change the global rule that currently calls anyRequest().permitAll() to anyRequest().authenticated() so JWT/OAuth2 enforcement applies by default; keep the explicit permitAll() matchers (requestMatchers("/", "/.well-known/**", "/css/**", "/*.ico", "/error", "/images/**"), permitAllRequest(), authRelatedEndpoints(), oauth2Endpoints(), actuatorEndpoints(), additionalSwaggerRequests()) to remain before the anyRequest() call so only those endpoints stay public; update the inline comment near anyRequest() to reflect authentication requirement instead of “지정하지 않은 url의 경우 인증 처리” if needed.
🧹 Nitpick comments (2)
@plan/grafana-alloy-minimal-plan.md (1)
31-40: 헤딩 레벨을 일관되게 맞춰주세요.
## 작업 범위아래에서### 1.다음에## 2.가 와서 문서 구조가 깨집니다.### 2.로 맞추는 편이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @@plan/grafana-alloy-minimal-plan.md around lines 31 - 40, The heading levels are inconsistent: after the "## 작업 범위" section there is "### 1." followed incorrectly by "## 2. 서버 준비"; change the incorrect "## 2. 서버 준비" (and any subsequent "## 2." headings in this area) to "### 2. 서버 준비" so all subitems under "## 작업 범위" use the same third-level heading style, and ensure "## 3. 최소 Alloy 설정" remains at the intended second level.@plan/prometheus-endpoint-plan.md (1)
8-14:현재 상태 요약이 PR 결과와 충돌하지 않게 갱신해 주세요.해당 섹션은 “의존성/management/security 설정이 아직 없다”는 현재형 표현이라, 이 PR 머지 후 문서 신뢰도를 떨어뜨릴 수 있습니다. “변경 전 상태”로 명시하거나 최신 상태로 업데이트하는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @@plan/prometheus-endpoint-plan.md around lines 8 - 14, Update the "현재 상태 요약" section so it does not assert an outdated present-tense state after this PR; either rename it to "변경 전 상태" and keep the bullets describing pre-change conditions (references: build.gradle dependency absence, Dockerfile/docker-compose healthcheck using /actuator/health, application-local.yml management and server.servlet.context-path, missing management.* in application.yml, and SecurityConfig lacking explicit actuator policy) or edit each bullet to reflect the post-PR reality (e.g., note which dependencies and management/security entries were added) so the summary matches the PR result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.local.json:
- Around line 17-23: The config contains machine-specific and overly-broad
permissions: remove or replace the hardcoded entries like the
"Bash(JAVA_HOME=... ./gradlew...)" and the PowerShell "Bash(powershell ...
GetFolder ... ShortPath)" lines, and tighten the broad "Read(//c/Program
Files/**)" / "Read(//c/Program Files/Java/**)" patterns; instead scope read
permissions to repo-relative paths (e.g., project directories) or use
environment variables (e.g., allow JAVA_HOME without embedding a full local
path) so the file is environment-agnostic and follows least-privilege
principles.
In @.omc/state/checkpoints/checkpoint-2026-03-30T01-42-12-096Z.json:
- Around line 1-16: Delete the committed runtime checkpoint artifact
.omc/state/checkpoints/checkpoint-2026-03-30T01-42-12-096Z.json from the PR and
stop tracking similar files by adding the .omc/ (or at minimum .omc/state/ or
.omc/state/checkpoints/) pattern to .gitignore; if the file is already tracked
run git rm --cached
.omc/state/checkpoints/checkpoint-2026-03-30T01-42-12-096Z.json (or git rm -r
--cached .omc/), commit the removal with a clear message, and push the branch.
If you need to preserve this runtime state locally, keep a local copy outside
the repo before removing.
In @@plan/prometheus-endpoint-plan.md:
- Line 1: The file contains a UTF-8 BOM character at the start (the invisible
U+FEFF) which should be removed; open the file referenced by the diff and
re-save it as UTF-8 without BOM (remove the leading BOM character from the first
line "# KoSpot Backend Prometheus Endpoint Plan...") so the file starts with a
normal '#' and avoid introducing any other content or formatting changes.
In `@docs/deploy-docs/alloy/config.alloy.example`:
- Around line 25-30: The Alloy config uses unsupported string interpolation for
environment vars; replace the "${...}" usages with sys.env("...") calls for the
prometheus.remote_write url and basic_auth fields—specifically update the url
entry (currently referencing GRAFANA_CLOUD_PROM_RW_URL) and the
basic_auth.username and basic_auth.password entries (currently referencing
GRAFANA_CLOUD_PROM_USER and GRAFANA_CLOUD_API_KEY) to call
sys.env("GRAFANA_CLOUD_PROM_RW_URL"), sys.env("GRAFANA_CLOUD_PROM_USER"), and
sys.env("GRAFANA_CLOUD_API_KEY") respectively so the Alloy parser can read the
environment variables.
In `@docs/plan-issue-200-email-auth.md`:
- Line 13: Several fenced code blocks in the markdown lack language identifiers
which violates MD040; open the document and update each triple-backtick fence
(the fenced code blocks currently like ``` ) to include an explicit language tag
(e.g., ```bash, ```sh, ```json, ```yaml, ```js) appropriate to the block content
so markdownlint passes; locate the fences around the example blocks noted in the
review and add the correct language identifiers to each.
- Around line 420-423: Replace the distinct EMAIL_NOT_FOUND enum/response with a
single generic credential error so unknown email and wrong password cannot be
distinguished: remove or stop using EMAIL_NOT_FOUND and instead return a unified
error (e.g., INVALID_CREDENTIALS) with the same HTTP status (UNAUTHORIZED) and
message used for INVALID_PASSWORD; update the login/auth path (e.g.,
authenticateUser / authService.login / the login handler) so both "email not
found" and "password mismatch" map to INVALID_CREDENTIALS and ensure
EMAIL_ALREADY_EXISTS remains unchanged for registration flows.
In `@docs/plan-issue-202-password-reset.md`:
- Line 37: Multiple fenced code blocks in this document lack language
identifiers (triggering markdownlint MD040); update each triple-backtick fence
to include an appropriate language specifier (for example use `text` for plain
output, `yaml` for YAML snippets, `sql` for SQL, `java` for Java, etc.) so every
code fence in the file (including the blocks flagged around the comment) becomes
```<language> ... ``` rather than plain ```, ensuring consistency across all
reported positions.
- Around line 653-654: The statement "last link only works" is inaccurate given
the current per-token UUID model; either update the doc text to state that
multiple links remain valid until TTL expiry or use, or implement server-side
revocation: modify the password reset flow (e.g., in
issuePasswordResetToken(memberId) or createPasswordResetToken) to call a new
invalidatePreviousResetTokens(memberId) / revokeOlderTokensForMember before
inserting the new per-token UUID, and ensure token validation
(validatePasswordResetToken) checks a per-member "current token" or revoked flag
(or enforces single active token) and respects passwordResetTTL; update the doc
accordingly to reflect the chosen approach.
- Around line 39-40: The Redis key pattern currently uses raw email
("password-reset-rate:{email}") which exposes PII; change key generation to use
a deterministic hash of the email (e.g., SHA-256) combined with a server-side
pepper, and use that hash in place of the raw email for both writes and reads.
Implement a helper like hashEmailWithPepper(email, PEPPER) that reads PEPPER
from secure config/env, produces a hex/base64 digest, and use the result when
constructing the "password-reset-rate:{hash}" key; update all places that create
or read that key (the code creating "password-reset-rate:{...}" and the
corresponding lookup/TTL/expiry logic referenced in the diff and the other
occurrences) so hashing is applied consistently. Ensure the pepper is not
checked into source control and add tests verifying hashed-key creation and
lookup behave identically to the previous flow.
In `@src/main/resources/application.yml`:
- Around line 87-101: The Prometheus actuator endpoint is currently exposed via
management.endpoints.web.exposure.include (health, prometheus) and must be
restricted; update configuration to avoid exposing /actuator/prometheus on the
public app port by either removing "prometheus" from
management.endpoints.web.exposure.include and enabling it only on a separate
management port (set management.server.port to a non-public port and/or adjust
management.server.address), or keep it but place the management endpoints behind
an authenticated/scrape-only path (use management.endpoints.web.base-path) and
enforce security rules (Spring Security config) or network ACLs so only internal
collectors can reach /actuator/prometheus; reference
management.endpoints.web.exposure.include, /actuator/prometheus,
management.server.port, and management.endpoints.web.base-path when making the
change.
---
Outside diff comments:
In @.omc/state/mission-state.json:
- Around line 1-145: The repo is committing auto-generated tool state
(.omc/state/mission-state.json); update the repository ignore rules by adding
the .omc/ directory to .gitignore so all files under .omc (e.g.,
.omc/state/mission-state.json) are excluded from version control, commit the
updated .gitignore, and remove any already-tracked .omc files from the index
(git rm --cached) before committing to ensure the directory is ignored going
forward.
In `@src/main/java/com/kospot/common/security/config/SecurityConfig.java`:
- Around line 86-99: In configureAuth(HttpSecurity) change the global rule that
currently calls anyRequest().permitAll() to anyRequest().authenticated() so
JWT/OAuth2 enforcement applies by default; keep the explicit permitAll()
matchers (requestMatchers("/", "/.well-known/**", "/css/**", "/*.ico", "/error",
"/images/**"), permitAllRequest(), authRelatedEndpoints(), oauth2Endpoints(),
actuatorEndpoints(), additionalSwaggerRequests()) to remain before the
anyRequest() call so only those endpoints stay public; update the inline comment
near anyRequest() to reflect authentication requirement instead of “지정하지 않은 url의
경우 인증 처리” if needed.
---
Nitpick comments:
In @@plan/grafana-alloy-minimal-plan.md:
- Around line 31-40: The heading levels are inconsistent: after the "## 작업 범위"
section there is "### 1." followed incorrectly by "## 2. 서버 준비"; change the
incorrect "## 2. 서버 준비" (and any subsequent "## 2." headings in this area) to
"### 2. 서버 준비" so all subitems under "## 작업 범위" use the same third-level heading
style, and ensure "## 3. 최소 Alloy 설정" remains at the intended second level.
In @@plan/prometheus-endpoint-plan.md:
- Around line 8-14: Update the "현재 상태 요약" section so it does not assert an
outdated present-tense state after this PR; either rename it to "변경 전 상태" and
keep the bullets describing pre-change conditions (references: build.gradle
dependency absence, Dockerfile/docker-compose healthcheck using
/actuator/health, application-local.yml management and
server.servlet.context-path, missing management.* in application.yml, and
SecurityConfig lacking explicit actuator policy) or edit each bullet to reflect
the post-PR reality (e.g., note which dependencies and management/security
entries were added) so the summary matches the PR result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8ffeecd-eef9-4910-b7de-951a9f85d7c3
📒 Files selected for processing (25)
.claude/settings.local.json.omc/project-memory.json.omc/sessions/2be8f18b-a303-4354-a119-67d147513c2b.json.omc/sessions/a7369daa-1159-421a-b86b-e3734b073ebb.json.omc/sessions/d961b845-8ef1-44ea-9218-076316d69f03.json.omc/sessions/fb01eec4-6632-4005-9c26-dd1c8b0ea9bf.json.omc/state/agent-replay-ac6f9e32-9736-4b54-bcac-e68e15837a3d.jsonl.omc/state/agent-replay-c227e4b0-8f79-4292-8662-03979cfcc0fb.jsonl.omc/state/checkpoints/checkpoint-2026-03-30T01-42-12-096Z.json.omc/state/idle-notif-cooldown.json.omc/state/last-tool-error.json.omc/state/mission-state.json.omc/state/subagent-tracking.json@plan/grafana-alloy-minimal-plan.md@plan/prometheus-endpoint-plan.mdKoSpot-backend-privatebuild.gradledocs/deploy-docs/GRAFANA_ALLOY_MINIMAL_SETUP.mddocs/deploy-docs/alloy/alloy.env.exampledocs/deploy-docs/alloy/alloy.service.override.exampledocs/deploy-docs/alloy/config.alloy.exampledocs/plan-issue-200-email-auth.mddocs/plan-issue-202-password-reset.mdsrc/main/java/com/kospot/common/security/config/SecurityConfig.javasrc/main/resources/application.yml
💤 Files with no reviewable changes (3)
- .omc/state/subagent-tracking.json
- .omc/state/agent-replay-ac6f9e32-9736-4b54-bcac-e68e15837a3d.jsonl
- .omc/state/agent-replay-c227e4b0-8f79-4292-8662-03979cfcc0fb.jsonl
Comment on lines
+17
to
+23
| "Read(//c/Program Files/Java/**)", | ||
| "Read(//c/Program Files/**)", | ||
| "Read(//c/Program Files/Java//**)", | ||
| "Read(//c/Users/이승영/.jdks/**)", | ||
| "Bash(JAVA_HOME=\"C:/Users/이승영/.jdks/jdk-17\" ./gradlew build -x test --no-daemon)", | ||
| "Bash(JAVA_HOME=\"/c/Users/이승영/.jdks/jdk-17\" ./gradlew build -x test --no-daemon)", | ||
| "Bash(powershell -Command \"\\(New-Object -ComObject Scripting.FileSystemObject\\).GetFolder\\('C:\\\\Users\\\\이승영\\\\.jdks\\\\jdk-17'\\).ShortPath\")" |
There was a problem hiding this comment.
Machine-specific and over-broad permissions should be narrowed.
These entries hardcode a personal Windows path (Line 20–23) and grant very broad read access (Line 18), which makes this config non-portable and weakens least-privilege. Please keep committed permissions environment-agnostic and scoped to repo-required paths only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings.local.json around lines 17 - 23, The config contains
machine-specific and overly-broad permissions: remove or replace the hardcoded
entries like the "Bash(JAVA_HOME=... ./gradlew...)" and the PowerShell
"Bash(powershell ... GetFolder ... ShortPath)" lines, and tighten the broad
"Read(//c/Program Files/**)" / "Read(//c/Program Files/Java/**)" patterns;
instead scope read permissions to repo-relative paths (e.g., project
directories) or use environment variables (e.g., allow JAVA_HOME without
embedding a full local path) so the file is environment-agnostic and follows
least-privilege principles.
Comment on lines
+1
to
+16
| { | ||
| "created_at": "2026-03-30T01:42:12.096Z", | ||
| "trigger": "manual", | ||
| "active_modes": {}, | ||
| "todo_summary": { | ||
| "pending": 0, | ||
| "in_progress": 0, | ||
| "completed": 0 | ||
| }, | ||
| "wisdom_exported": false, | ||
| "background_jobs": { | ||
| "active": [], | ||
| "recent": [], | ||
| "stats": null | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Remove committed runtime checkpoint artifact from the PR.
This .omc checkpoint is environment/tool state, not product code/config tied to Actuator/Prometheus. Keeping it in git will create noise and frequent conflicts.
Suggested cleanup
- .omc/state/checkpoints/checkpoint-2026-03-30T01-42-12-096Z.json+# Ignore local OMC runtime state
+.omc/state/
+.omc/sessions/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.omc/state/checkpoints/checkpoint-2026-03-30T01-42-12-096Z.json around lines
1 - 16, Delete the committed runtime checkpoint artifact
.omc/state/checkpoints/checkpoint-2026-03-30T01-42-12-096Z.json from the PR and
stop tracking similar files by adding the .omc/ (or at minimum .omc/state/ or
.omc/state/checkpoints/) pattern to .gitignore; if the file is already tracked
run git rm --cached
.omc/state/checkpoints/checkpoint-2026-03-30T01-42-12-096Z.json (or git rm -r
--cached .omc/), commit the removal with a clear message, and push the branch.
If you need to preserve this runtime state locally, keep a local copy outside
the repo before removing.
| @@ -0,0 +1,81 @@ | |||
| # KoSpot Backend Prometheus Endpoint Plan (Grafana Cloud 연동 전제) | |||
There was a problem hiding this comment.
문서 파일 BOM 문자 제거 권장.
Line 1 시작의 BOM()은 일부 도구에서 diff/렌더링 잡음을 만듭니다. UTF-8 without BOM으로 저장해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @@plan/prometheus-endpoint-plan.md at line 1, The file contains a UTF-8 BOM
character at the start (the invisible U+FEFF) which should be removed; open the
file referenced by the diff and re-save it as UTF-8 without BOM (remove the
leading BOM character from the first line "# KoSpot Backend Prometheus Endpoint
Plan...") so the file starts with a normal '#' and avoid introducing any other
content or formatting changes.
Comment on lines
+25
to
+30
| url = "${GRAFANA_CLOUD_PROM_RW_URL}" | ||
|
|
||
| basic_auth { | ||
| username = "${GRAFANA_CLOUD_PROM_USER}" | ||
| password = "${GRAFANA_CLOUD_API_KEY}" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Grafana Alloy config syntax for environment variables: Is "${VAR}" supported in config.alloy, or should env("VAR") / sys.env("VAR") be used for prometheus.remote_write url and basic_auth fields?
💡 Result:
No, "${VAR}" is not supported in Grafana Alloy config.alloy files. Use sys.env("VAR") to reference environment variables. For prometheus.remote_write url and basic_auth fields, use sys.env("VAR") as shown in official examples: prometheus.remote_write "default" { endpoint { url = "https://prometheus-xxx.grafana.net/api/prom/push" basic_auth { username = sys.env("PROMETHEUS_USERNAME") password = sys.env("GRAFANA_CLOUD_API_KEY") } } } Using "${GRAFANA_CLOUD_KEY}" directly causes parse errors, as confirmed in GitHub issue where it was corrected to sys.env. There is no env("VAR"); it's sys.env("VAR"). If the variable doesn't exist, sys.env returns an empty string.
Citations:
- 1: https://grafana.com/docs/alloy/latest/get-started/configuration-syntax/syntax/
- 2: https://grafana.com/docs/alloy/latest/get-started/syntax/
- 3: Grafana Alloy fails to parse environment variable defined in /etc/default/alloy grafana/alloy#2246
- 4: https://grafana.com/docs/alloy/latest/reference/components/prometheus/prometheus.remote_write/
- 5: https://grafana.com/docs/alloy/latest/reference/stdlib/sys
- 6: https://grafana.com/docs/alloy/latest/get-started/configuration-syntax/expressions/function_calls/
Replace string interpolation with sys.env() function calls.
"${...}" syntax is not supported in Alloy configuration and will cause parse errors. Use sys.env("VAR") to reference environment variables in prometheus.remote_write url and basic_auth fields.
Required fix
prometheus.remote_write "grafana_cloud" {
endpoint {
- url = "${GRAFANA_CLOUD_PROM_RW_URL}"
+ url = sys.env("GRAFANA_CLOUD_PROM_RW_URL")
basic_auth {
- username = "${GRAFANA_CLOUD_PROM_USER}"
- password = "${GRAFANA_CLOUD_API_KEY}"
+ username = sys.env("GRAFANA_CLOUD_PROM_USER")
+ password = sys.env("GRAFANA_CLOUD_API_KEY")
}
}
}📝 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
| url = "${GRAFANA_CLOUD_PROM_RW_URL}" | |
| basic_auth { | |
| username = "${GRAFANA_CLOUD_PROM_USER}" | |
| password = "${GRAFANA_CLOUD_API_KEY}" | |
| } | |
| url = sys.env("GRAFANA_CLOUD_PROM_RW_URL") | |
| basic_auth { | |
| username = sys.env("GRAFANA_CLOUD_PROM_USER") | |
| password = sys.env("GRAFANA_CLOUD_API_KEY") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/deploy-docs/alloy/config.alloy.example` around lines 25 - 30, The Alloy
config uses unsupported string interpolation for environment vars; replace the
"${...}" usages with sys.env("...") calls for the prometheus.remote_write url
and basic_auth fields—specifically update the url entry (currently referencing
GRAFANA_CLOUD_PROM_RW_URL) and the basic_auth.username and basic_auth.password
entries (currently referencing GRAFANA_CLOUD_PROM_USER and
GRAFANA_CLOUD_API_KEY) to call sys.env("GRAFANA_CLOUD_PROM_RW_URL"),
sys.env("GRAFANA_CLOUD_PROM_USER"), and sys.env("GRAFANA_CLOUD_API_KEY")
respectively so the Alloy parser can read the environment variables.
|
|
||
| ### 기존 인증 흐름 | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Specify code fence languages to satisfy markdownlint.
The flagged fenced blocks should include explicit languages (for MD040 compliance).
Also applies to: 215-215, 243-243, 434-434
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plan-issue-200-email-auth.md` at line 13, Several fenced code blocks in
the markdown lack language identifiers which violates MD040; open the document
and update each triple-backtick fence (the fenced code blocks currently like ```
) to include an explicit language tag (e.g., ```bash, ```sh, ```json, ```yaml,
```js) appropriate to the block content so markdownlint passes; locate the
fences around the example blocks noted in the review and add the correct
language identifiers to each.
Comment on lines
+420
to
+423
| EMAIL_ALREADY_EXISTS(CONFLICT, 4102, "이미 사용 중인 이메일입니다."), | ||
| EMAIL_NOT_FOUND(UNAUTHORIZED, 4103, "존재하지 않는 이메일입니다."), | ||
| // ※ NOT_FOUND(404) 대신 UNAUTHORIZED(401) 사용 — 이메일 존재 여부 노출 방지 (보안) | ||
| INVALID_PASSWORD(UNAUTHORIZED, 4104, "비밀번호가 일치하지 않습니다."), |
There was a problem hiding this comment.
Current login error design still enables email enumeration.
Using a dedicated EMAIL_NOT_FOUND message reveals account existence even with HTTP 401. For login, return a single generic credential error message for both unknown email and wrong password.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plan-issue-200-email-auth.md` around lines 420 - 423, Replace the
distinct EMAIL_NOT_FOUND enum/response with a single generic credential error so
unknown email and wrong password cannot be distinguished: remove or stop using
EMAIL_NOT_FOUND and instead return a unified error (e.g., INVALID_CREDENTIALS)
with the same HTTP status (UNAUTHORIZED) and message used for INVALID_PASSWORD;
update the login/auth path (e.g., authenticateUser / authService.login / the
login handler) so both "email not found" and "password mismatch" map to
INVALID_CREDENTIALS and ensure EMAIL_ALREADY_EXISTS remains unchanged for
registration flows.
|
|
||
| ### Redis 키 설계 | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
markdownlint MD040 is triggered in multiple sections; please specify fence languages (e.g., text, java, yaml, sql) for each block.
Also applies to: 49-49, 60-60, 88-88, 192-192, 249-249, 270-270, 313-313, 340-340, 363-363, 395-395, 447-447, 489-489, 581-581
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 37-37: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plan-issue-202-password-reset.md` at line 37, Multiple fenced code
blocks in this document lack language identifiers (triggering markdownlint
MD040); update each triple-backtick fence to include an appropriate language
specifier (for example use `text` for plain output, `yaml` for YAML snippets,
`sql` for SQL, `java` for Java, etc.) so every code fence in the file (including
the blocks flagged around the comment) becomes ```<language> ... ``` rather than
plain ```, ensuring consistency across all reported positions.
Comment on lines
+39
to
+40
| password-reset-rate:{email} → {count} TTL: 1시간 (요청 횟수 제한) | ||
| ``` |
There was a problem hiding this comment.
Avoid storing raw email in Redis keys.
Using plain email in key names increases PII exposure through logs/ops tooling. Prefer a deterministic hash (e.g., SHA-256 with a server-side pepper) for password-reset-rate:{...} keys.
Also applies to: 297-303
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plan-issue-202-password-reset.md` around lines 39 - 40, The Redis key
pattern currently uses raw email ("password-reset-rate:{email}") which exposes
PII; change key generation to use a deterministic hash of the email (e.g.,
SHA-256) combined with a server-side pepper, and use that hash in place of the
raw email for both writes and reads. Implement a helper like
hashEmailWithPepper(email, PEPPER) that reads PEPPER from secure config/env,
produces a hex/base64 digest, and use the result when constructing the
"password-reset-rate:{hash}" key; update all places that create or read that key
(the code creating "password-reset-rate:{...}" and the corresponding
lookup/TTL/expiry logic referenced in the diff and the other occurrences) so
hashing is applied consistently. Ensure the pepper is not checked into source
control and add tests verifying hashed-key creation and lookup behave
identically to the previous flow.
Comment on lines
+653
to
+654
| 기존 토큰을 명시적으로 무효화하지 않는다. 새 요청마다 새 토큰이 발급되며, 마지막으로 받은 링크만 실제 사용 가능하다. Rate Limit으로 무한 발급을 제한. | ||
|
|
There was a problem hiding this comment.
“Only the last link works” is incorrect for the current token model.
With per-token UUID keys and no explicit invalidation of prior tokens, multiple issued links remain valid until TTL expiry or use. Please fix this statement or add logic that revokes older tokens per member.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plan-issue-202-password-reset.md` around lines 653 - 654, The statement
"last link only works" is inaccurate given the current per-token UUID model;
either update the doc text to state that multiple links remain valid until TTL
expiry or use, or implement server-side revocation: modify the password reset
flow (e.g., in issuePasswordResetToken(memberId) or createPasswordResetToken) to
call a new invalidatePreviousResetTokens(memberId) / revokeOlderTokensForMember
before inserting the new per-token UUID, and ensure token validation
(validatePasswordResetToken) checks a per-member "current token" or revoked flag
(or enforces single active token) and respects passwordResetTTL; update the doc
accordingly to reflect the chosen approach.
Comment on lines
+87
to
+101
| management: | ||
| endpoints: | ||
| web: | ||
| exposure: | ||
| include: health, prometheus | ||
| endpoint: | ||
| health: | ||
| show-details: never | ||
| prometheus: | ||
| metrics: | ||
| export: | ||
| enabled: true | ||
| metrics: | ||
| tags: | ||
| application: kospot-backend |
There was a problem hiding this comment.
Prometheus endpoint exposure needs tighter access control.
management.endpoints.web.exposure.include: health, prometheus is fine for scraping, but /actuator/prometheus should be restricted to internal collectors (network ACL / separate management port / authenticated scrape path). In the current security posture, this risks broad runtime metadata disclosure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/application.yml` around lines 87 - 101, The Prometheus
actuator endpoint is currently exposed via
management.endpoints.web.exposure.include (health, prometheus) and must be
restricted; update configuration to avoid exposing /actuator/prometheus on the
public app port by either removing "prometheus" from
management.endpoints.web.exposure.include and enabling it only on a separate
management port (set management.server.port to a non-public port and/or adjust
management.server.address), or keep it but place the management endpoints behind
an authenticated/scrape-only path (use management.endpoints.web.base-path) and
enforce security rules (Spring Security config) or network ACLs so only internal
collectors can reach /actuator/prometheus; reference
management.endpoints.web.exposure.include, /actuator/prometheus,
management.server.port, and management.endpoints.web.base-path when making the
change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit