Skip to content

Strix PR 스코프에서 Next.js 동적 라우트 경로 허용 - #200

Closed
seonghobae wants to merge 2 commits into
masterfrom
fix/strix-nextjs-bracket-paths-20260514
Closed

Strix PR 스코프에서 Next.js 동적 라우트 경로 허용#200
seonghobae wants to merge 2 commits into
masterfrom
fix/strix-nextjs-bracket-paths-20260514

Conversation

@seonghobae

@seonghobae seonghobae commented May 14, 2026

Copy link
Copy Markdown
Contributor

No linked issue.

목적

pull_request_target 기반 Strix 스코프 검증이 frontend/src/app/labels/[slug]/page.tsx 같은 Next.js 동적 라우트 경로를 잘못 거부해 필수 Strix 스캔이 시작 전 실패하는 문제를 수정합니다.

주요 변경

  • 변경 파일 경로 정규화 regex에서 리터럴 [ / ]를 허용합니다.
  • : pathspec magic, *, traversal, absolute path, backslash, 앞뒤 공백은 계속 차단합니다.
  • symlink/non-regular file 차단과 PR-head blob read 실패 시 fail-closed 동작은 기존 검사로 유지합니다.
  • pull-request-target-nextjs-bracket-route-uses-head-blob 회귀 케이스를 추가했습니다.

검증

  • bash scripts/ci/test_strix_quick_gate.sh: PASS
  • git diff --check: PASS
  • python3 "$HOME/.config/opencode/scripts/lint_by_filetype.py" --json: ok

Summary by CodeRabbit

  • Chores

    • CI path validation now accepts parameterized paths that include square brackets (e.g., framework-style dynamic routes).
    • Target-path resolution in the CI gate is stricter: unresolved paths now cause validation to fail rather than be lenient.
  • Tests

    • Added coverage for dynamic/parameterized route files to ensure correct scanning during pull-request validation.
    • Strengthened self-tests to assert strict path resolution behavior.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'version'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1ff0458f-8f0e-4a5f-be9d-fafdc2831c70

📥 Commits

Reviewing files that changed from the base of the PR and between 9d63ab2 and 613cc58.

📒 Files selected for processing (2)
  • scripts/ci/strix_quick_gate.sh
  • scripts/ci/test_strix_quick_gate.sh

📝 Walkthrough

Walkthrough

Adds bracket characters ([ and ]) to allowed normalized changed-file paths and switches target-path resolution to strict (Path.resolve(strict=True)); tests updated to assert strict resolution is used and to cover a Next.js bracket-route path.

Changes

Path handling and resolution changes

Layer / File(s) Summary
Normalize changed-file paths to allow brackets
scripts/ci/strix_quick_gate.sh
normalize_changed_file_path() expands its re.fullmatch() whitelist to include [ and ], allowing parameterized Next.js route paths.
Make target path resolution strict
scripts/ci/strix_quick_gate.sh
resolve_scan_target_path() and resolve_internal_pr_scope_target_path() call Path.resolve(strict=True), so unresolved targets now cause resolution errors.
Test assertions for strict resolution and bracket routes
scripts/ci/test_strix_quick_gate.sh
Adds an assertion that the gate script does not use target_path.resolve(strict=False) and a run_pull_request_target_head_scope_case for frontend/src/app/labels/[slug]/page.tsx validating head-only scanning.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I rabbit-hop through bracketed lanes,
Where [slug] and friends elude old chains,
Resolve stands strict and paths behave—
Tests guard the gate, the scanner's brave. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Strix PR 스코프에서 Next.js 동적 라우트 경로 허용' is in Korean and directly describes the main change: allowing Next.js dynamic route paths in Strix PR scope validation.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/strix-nextjs-bracket-paths-20260514

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

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 9d63ab2456340e75e04543b52b712da84da62778:

  • 3 required check(s) are not successful on 9d63ab2.\n- Missing current-head CodeRabbit/coderabbitai evidence for 9d63ab2.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 9d63ab2456340e75e04543b52b712da84da62778:

  • 2 required check(s) are not successful on 9d63ab2.\n- Missing current-head CodeRabbit/coderabbitai evidence for 9d63ab2.\n

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 9d63ab2456340e75e04543b52b712da84da62778:

  • 2 required check(s) are not successful on 9d63ab2.\n- Missing current-head CodeRabbit/coderabbitai evidence for 9d63ab2.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 9d63ab2456340e75e04543b52b712da84da62778:

  • 1 required check(s) are not successful on 9d63ab2.\n- Missing current-head CodeRabbit/coderabbitai evidence for 9d63ab2.\n

@greptile-apps

greptile-apps Bot commented May 14, 2026

Copy link
Copy Markdown

Greptile Summary

경로 정규화 regex에서 리터럴 [·]를 허용하여 Next.js 동적 라우트 경로([slug] 등)가 regex 검증 단계에서 거부되던 버그를 수정합니다. 함께 포함된 strict=Falsestrict=True 변경은 scan target 경로 해석을 더욱 엄격하게 만드는 보안 강화입니다.

  • regex 수정 ([A-Za-z0-9_./ -]+[A-Za-z0-9_./ \\[\\]-]+): \\[·\\]를 추가하고, -는 character class 끝에 위치시켜 literal로 유지합니다.
  • strict=True 강화: resolve_scan_target_path·resolve_internal_pr_scope_target_path 두 함수에서 존재하지 않는 경로를 Python 레벨에서 즉시 거부합니다.
  • 회귀 테스트: pull-request-target-nextjs-bracket-route-uses-head-blob 케이스로 실제 git repo를 생성해 end-to-end 검증합니다.

Confidence Score: 5/5

변경 범위가 작고 기존 방어 로직(tree_path 일치 검사, fail-closed 동작)이 그대로 유지되어 안전하게 머지 가능합니다.

regex 확장은 [, ] 두 문자만 추가하고 -는 기존과 동일하게 character class 끝에서 literal로 처리됩니다. git ls-tree 결과의 경로 일치 검사(line 220)가 glob 오해석 가능성을 방어하며, git show HEAD:path는 literal 경로를 사용합니다. strict=True 변경은 존재하지 않는 경로에 대해 빠른 실패를 보장하는 순수 강화입니다.

별도로 주의가 필요한 파일 없음.

Important Files Changed

Filename Overview
scripts/ci/strix_quick_gate.sh 정규화 regex에 \[, \] 이스케이프 추가, resolve_scan_target_path 및 resolve_internal_pr_scope_target_path 두 곳의 strict=False → strict=True 보안 강화.
scripts/ci/test_strix_quick_gate.sh [slug] 브라켓 라우트 회귀 케이스 추가 및 strict=False 미사용 구조적 검증 assertion 추가.

Reviews (2): Last reviewed commit: "fix: harden Strix scan target resolution" | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 613cc58c475b4f15bd06368ff923c5412500d461:

  • 3 required check(s) are not successful on 613cc58.\n- Missing current-head CodeRabbit/coderabbitai evidence for 613cc58.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 613cc58c475b4f15bd06368ff923c5412500d461:

  • 2 required check(s) are not successful on 613cc58.\n- Missing current-head CodeRabbit/coderabbitai evidence for 613cc58.\n

1 similar comment
@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 613cc58c475b4f15bd06368ff923c5412500d461:

  • 2 required check(s) are not successful on 613cc58.\n- Missing current-head CodeRabbit/coderabbitai evidence for 613cc58.\n

@github-actions

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 613cc58c475b4f15bd06368ff923c5412500d461:

  • 1 required check(s) are not successful on 613cc58.\n- Missing current-head CodeRabbit/coderabbitai evidence for 613cc58.\n

@seonghobae

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae
seonghobae enabled auto-merge May 14, 2026 20:04
@seonghobae

Copy link
Copy Markdown
Contributor Author

Stale: Strix fixes already landed on master via #206-#208. This PR has conflicts.

@seonghobae seonghobae closed this May 24, 2026
auto-merge was automatically disabled May 24, 2026 00:37

Pull request was closed

@seonghobae seonghobae reopened this May 24, 2026
@github-actions

github-actions Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for 613cc58c475b4f15bd06368ff923c5412500d461:

@seonghobae

Copy link
Copy Markdown
Contributor Author

Closing this PR as superseded by current master.

Current evidence:

  • The original purpose, allowing Next.js bracket dynamic route paths in Strix PR scope, is already present on origin/master.
  • scripts/ci/strix_quick_gate.sh on origin/master already allows bracket characters in normalized PR-head paths.
  • scripts/ci/test_strix_quick_gate.sh on origin/master already includes the pull-request-target-nextjs-bracket-route-uses-head-blob regression coverage.
  • The remaining branch diff is stale and the PR is currently dirty/conflicting.

No unique implementation needs to be preserved from this PR.

@seonghobae seonghobae closed this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant