fix(security): constrain artifact-link purposes on current main - #415
fix(security): constrain artifact-link purposes on current main#415seonghobae wants to merge 7 commits into
Conversation
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesArtifact 목적 검증
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The PR constrains artifact-link purposes at the request boundary while preserving documented defaults and legacy mapping; invalid purposes are rejected before signing. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
|
@opencode-agent review Please review the unchanged exact current head |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java`:
- Around line 53-58: Update cleanPurpose in ArtifactLinkRequest to reject values
containing a NUL character before applying strip(), throwing an exception with
the message "artifact link purpose contains NUL" instead of removing the
character. Preserve the existing null handling and empty-value conversion.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 913b1d64-58a6-47d1-8743-dc34fc7faa6b
📒 Files selected for processing (2)
src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.javasrc/test/java/com/clearfolio/viewer/api/ArtifactLinkRequestTest.java
|
Exact-current-head read-only review request for @opencode-agent review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='00814b6ea46c9b4e8783fd2e04924007c3fdd677'
base='961f44a19c4ced87f8062b3ab9842a9a08d05235'
repo='ContextualWisdomLab/clearfolio'
echo '== revision identity and ancestry =='
git rev-parse HEAD
git cat-file -t "$target"
git cat-file -t "$base"
git merge-base --is-ancestor "$base" "$target" && echo 'base_is_ancestor=yes'
git diff --name-status "$base" "$target"
git diff --check "$base" "$target"
echo '== patch =='
git diff --find-renames --find-copies "$base" "$target" -- \
src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java \
src/test/java/com/clearfolio/viewer/api/ArtifactLinkRequestTest.java
echo '== ArtifactLinkRequest outline =='
ast-grep outline src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java --items all --view expanded
echo '== ArtifactLinkRequest implementation =='
cat -n src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java
echo '== ArtifactLinkRequest tests =='
cat -n src/test/java/com/clearfolio/viewer/api/ArtifactLinkRequestTest.java
echo '== signing-related call sites =='
rg -n -C 5 --glob '*.java' \
'(ArtifactLinkRequest|purpose\(\)|getPurpose\(\)|sign[A-Za-z]*\(|SignedArtifact|artifact[-A-Za-z]*link)' \
src/main/java src/test/java
echo '== GitHub PR metadata and checks =='
gh pr view 415 --repo "$repo" --json number,headRefOid,baseRefName,baseRefOid,mergeCommit,statusCheckRollupLength of output: 143601 🏁 Script executed: #!/bin/bash
set -euo pipefail
target='00814b6ea46c9b4e8783fd2e04924007c3fdd677'
base='961f44a19c4ced87f8062b3ab9842a9a08d05235'
repo='ContextualWisdomLab/clearfolio'
echo '== remaining changed test delta =='
git diff --find-renames "$base" "$target" -- \
src/test/java/com/clearfolio/viewer/artifact/ArtifactLinkServiceTest.java
echo '== createLink, purpose fallback, and signing order =='
sed -n '120,185p' src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java
rg -n -C 8 --glob '*.java' 'String purposeOf\s*\(' src/main/java
echo '== controller link endpoint =='
sed -n '65,90p' src/main/java/com/clearfolio/viewer/controller/ArtifactController.java
echo '== controller tests for invalid request bodies =='
rg -n -C 8 --glob '*Test.java' \
'(admin-export|\\\\u0000|NUL|unsupported|artifact link purpose)' src/test/java
echo '== purpose contract in repository documentation and API schemas =='
rg -n -i -C 4 \
'(viewer-preview|admin-export|artifact.{0,12}purpose|purpose.{0,12}artifact|integration|download)' \
README.md docs src/main/resources 2>/dev/null || true
echo '== live main and protection metadata =='
git ls-remote origin refs/heads/main
gh api "repos/$repo/branches/main" --jq '{name,commit:.commit.sha,protected,protection_url}'
gh api "repos/$repo/branches/main/protection" \
--jq '{required_status_checks,required_pull_request_reviews,enforce_admins,restrictions}' 2>&1 || true
echo '== PR state, review decision, and review threads =='
gh pr view 415 --repo "$repo" --json state,isDraft,reviewDecision,reviews,commentsLength of output: 127473
No actionable findings were identified. Verified:
The detailed branch-protection configuration could not be read because the GitHub integration returned HTTP 403. GitHub confirms that
|
|
@opencode-agent review Review exact head Review read-only; preserve independent reviewer identity. Do not mutate the branch, synthesize author approval, weaken protection, or merge. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='00814b6ea46c9b4e8783fd2e04924007c3fdd677'
repo='ContextualWisdomLab/clearfolio'
echo '== exact revision =='
echo -n 'HEAD='; git rev-parse HEAD
echo -n 'target='; git rev-parse "$target"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$target"
echo '== PR and checks =='
gh pr view 415 --repo "$repo" --json number,headRefOid,baseRefName,baseRefOid,state,isDraft,reviewDecision,statusCheckRollup
gh api "repos/$repo/commits/$target/check-runs?per_page=100" --paginate \
--jq '.check_runs[] | [.name,.status,.conclusion,.details_url] | `@tsv`'
base="$(git show -s --format=%P "$target" | awk '{print $1}')"
echo "== parent base: $base =="
git merge-base --is-ancestor "$base" "$target"
git diff --check "$base" "$target"
git diff --name-status "$base" "$target"
git diff --find-renames "$base" "$target" -- \
src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java \
src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
src/test/java/com/clearfolio/viewer/api/ArtifactLinkRequestTest.java \
src/test/java/com/clearfolio/viewer/artifact/ArtifactLinkServiceTest.java
echo '== request type outline and source =='
ast-grep outline src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java --items all --view expanded
cat -n src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java
echo '== request tests =='
cat -n src/test/java/com/clearfolio/viewer/api/ArtifactLinkRequestTest.java
echo '== production request-to-signing paths =='
ast-grep outline src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java --items all --view expanded
rg -n -C 8 --glob '*.java' \
'(new ArtifactLinkRequest\s*\(|ArtifactLinkRequest\s*\(|\.purpose\(\)|getPurpose\(\)|purposeOf\s*\(|sign[A-Za-z]*\s*\()' \
src/main/java
echo '== relevant test coverage =='
rg -n -C 6 --glob '*Test.java' \
'(admin-export|viewer-preview|integration|download|viewer|u0000|NUL|unsupported|purpose)' \
src/test/javaLength of output: 202532 🏁 Script executed: #!/bin/bash
set -euo pipefail
log='/tmp/coderabbit-shell-logs/shell-output-p97u2F'
target='00814b6ea46c9b4e8783fd2e04924007c3fdd677'
echo '== captured revision, PR, checks, and diff summary =='
sed -n '1,260p' "$log"
echo '== selected exact-head source =='
cat -n src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java
printf '\n== ArtifactLinkService createLink and purpose extraction ==\n'
rg -n -A100 -B10 --glob '*.java' \
'ArtifactLinkResponse\s+createLink\s*\(|String\s+purposeOf\s*\(' \
src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java
printf '\n== exact request tests ==\n'
cat -n src/test/java/com/clearfolio/viewer/api/ArtifactLinkRequestTest.java
printf '\n== direct service tests for rejected purposes ==\n'
rg -n -A35 -B10 --glob '*Test.java' \
'(admin-export|unsupported artifact-link purpose|NUL|\\u0000)' \
src/test/java/com/clearfolio/viewer/artifact/ArtifactLinkServiceTest.java
echo '== all direct production construction sites =='
rg -n -C 3 --glob '*.java' \
'new\s+ArtifactLinkRequest\s*\(|ArtifactLinkRequest\.viewerPreview\s*\(' \
src/main/javaLength of output: 45530
No actionable findings were identified. Verified:
I did not approve the PR, modify the branch, alter protection, or merge.
|
|
Exact-current-head independent review request for The three-file artifact-link purpose boundary is mergeable; CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; the only inline finding is resolved on this head. Only @opencode-agent review Review this exact head read-only. Do not mutate the branch, synthesize author approval, weaken protection, or merge. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='ContextualWisdomLab/clearfolio'
target='00814b6ea46c9b4e8783fd2e04924007c3fdd677'
echo '== requested commit identity =='
git cat-file -t "$target"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$target"
parent="$(git show -s --format=%P "$target" | awk '{print $1}')"
printf 'first_parent=%s\n' "$parent"
echo '== live main and PR state =='
git ls-remote origin refs/heads/main
gh pr view 415 --repo "$repo" \
--json number,state,isDraft,headRefOid,baseRefName,baseRefOid,reviewDecision,statusCheckRollup
gh api "repos/$repo/branches/main" --jq '{name,protected,sha:.commit.sha}' || true
gh api "repos/$repo/commits/$target/check-runs?per_page=100" --paginate \
--jq '.check_runs[] | [.name,.status,.conclusion] | `@tsv`'
echo '== ancestry and exact delta =='
git merge-base --is-ancestor "$parent" "$target" && echo 'parent_is_ancestor=yes'
git diff --check "$parent" "$target"
git diff --name-status "$parent" "$target"
git diff --find-renames "$parent" "$target" -- \
src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java \
src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java \
src/test/java/com/clearfolio/viewer/api/ArtifactLinkRequestTest.java \
src/test/java/com/clearfolio/viewer/artifact/ArtifactLinkServiceTest.java
echo '== request boundary map and source =='
ast-grep outline src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java --items all --view expanded
cat -n src/main/java/com/clearfolio/viewer/api/ArtifactLinkRequest.java
echo '== request to signing path =='
rg -n -A100 -B10 --glob '*.java' \
'ArtifactLinkResponse\s+createLink\s*\(|String\s+purposeOf\s*\(' \
src/main/java/com/clearfolio/viewer/artifact/ArtifactLinkService.java
rg -n -C 5 --glob '*.java' \
'new\s+ArtifactLinkRequest\s*\(|ArtifactLinkRequest\.viewerPreview\s*\(|\.purpose\(\)|getPurpose\(\)|sign[A-Za-z]*\s*\(' \
src/main/java
echo '== focused tests =='
cat -n src/test/java/com/clearfolio/viewer/api/ArtifactLinkRequestTest.java
rg -n -A35 -B10 --glob '*Test.java' \
'(admin-export|viewer-preview|integration|download|unsupported artifact-link purpose|NUL|\\u0000)' \
src/test/java/com/clearfolio/viewer/artifact/ArtifactLinkServiceTest.javaLength of output: 46635
I found no new actionable source finding. Verified:
Live protected I did not modify the branch, approve the PR, change protection, or merge. ✅ Action performedReview finished.
|
1 participant
Objective
Constrain signed artifact-link purposes to the documented allowlist before any signing path consumes the request.
Exact current-head evidence
961f44a19c4ced87f8062b3ab9842a9a08d05235;e7e9b82f1782ed689f748eb09659d0550affaffe;00814b6ea46c9b4e8783fd2e04924007c3fdd677;Contract
viewer-preview,download, andintegrationare accepted; absent or blank purpose retains documented default behavior; legacyviewermaps only toviewer-preview; unknown purpose fails before signing. NUL-corrupted input is rejected rather than normalized into an allowed purpose.Scope boundary
Request-boundary validation only. Token signatures, TTLs, tenant authority, ledger persistence, delivery, and canonical documentation are unchanged.
Merge gate
Keep this exact head unchanged. Auto-merge may act only after live checks remain successful, zero valid unresolved findings remain, and a qualifying independent non-author approval is attached to this exact head. Automated evidence and predecessor reviews do not transfer.
Summary by CodeRabbit
viewer목적 값은viewer-preview로 자동 변환되어 호환성을 유지합니다.