Skip to content

fix(security): reconcile tenant-scoped admin deletion on current main - #436

Closed
seonghobae wants to merge 6 commits into
mainfrom
fix/admin-delete-current-main-20260813
Closed

fix(security): reconcile tenant-scoped admin deletion on current main#436
seonghobae wants to merge 6 commits into
mainfrom
fix/admin-delete-current-main-20260813

Conversation

@seonghobae

Copy link
Copy Markdown
Collaborator

Current-main replacement for #341. Exact head 3187b9b preserves the three-file tenant-scoped administrative deletion delta on protected main 961f44a. Regenerate all exact-head CI, security, SAST, fuzz, review, and independent approval evidence before merge.

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 118 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fd5b82f6-5f92-40f5-8b39-524b96a2fd60

📥 Commits

Reviewing files that changed from the base of the PR and between 961f44a and 3187b9b.

📒 Files selected for processing (3)
  • src/main/java/com/clearfolio/viewer/controller/AdminController.java
  • src/test/java/com/clearfolio/viewer/controller/AdminControllerDeleteTenantBoundaryTest.java
  • src/test/java/com/clearfolio/viewer/controller/AdminControllerTest.java

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.

@seonghobae
seonghobae marked this pull request as ready for review August 14, 2026 07:53
@seonghobae
seonghobae enabled auto-merge (squash) August 14, 2026 07:53

Copy link
Copy Markdown
Collaborator Author

Exact-current-head review request for 3187b9b5b1a2c9a8c9f9b27fbaee5808721daaef on protected main 961f44a19c4ced87f8062b3ab9842a9a08d05235. CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; the PR is mergeable and has no unresolved review threads. This branch is the current-main replacement for stale #341. Review read-only; do not mutate the branch or bypass independent approval.

@opencode-agent review
@cwl-noema-review review
@strix review
@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Repository state =='
git rev-parse HEAD
git status --short
git merge-base 3187b9b5b1a2c9a8c9f9b27fbaee5808721daaef 961f44a19c4ced87f8062b3ab9842a9a08d05235
git diff --stat 961f44a19c4ced87f8062b3ab9842a9a08d05235 3187b9b5b1a2c9a8c9f9b27fbaee5808721daaef
git diff --name-status 961f44a19c4ced87f8062b3ab9842a9a08d05235 3187b9b5b1a2c9a8c9f9b27fbaee5808721daaef
git diff --find-renames --find-copies --unified=80 \
  961f44a19c4ced87f8062b3ab9842a9a08d05235 \
  3187b9b5b1a2c9a8c9f9b27fbaee5808721daaef

echo '== Pull request metadata and checks =='
gh pr view 436 --repo ContextualWisdomLab/clearfolio \
  --json number,state,isDraft,mergeable,headRefOid,baseRefOid,reviewDecision,statusCheckRollup,reviews \
  --jq '{number,state,isDraft,mergeable,headRefOid,baseRefOid,reviewDecision,statusCheckRollup:[.statusCheckRollup[]|{name,conclusion,status}],reviews:[.reviews[]|{author:.author.login,state,commit:.commit.oid}]}'

echo '== Review threads =='
gh api graphql -F owner='ContextualWisdomLab' -F name='clearfolio' -F number=436 -f query='
query($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest(number: $number) {
      reviewThreads(first: 100) {
        nodes { isResolved isOutdated comments(first: 10) { nodes { author { login } body path line originalLine } } }
      }
    }
  }
}' --jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false)]'

Length of output: 20404


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Candidate source files =='
fd -t f -e java . src | rg '/(DocumentConversionService|TenantAccessService|TenantContext|ApiExceptionHandler|AdminController)\.java$'

echo '== Relevant declarations =='
ast-grep outline src/main/java --items all --type class,interface,method \
  --match 'DocumentConversionService|TenantAccessService|TenantContext|ApiExceptionHandler|AdminController|deleteJob|require'

echo '== Tenant-scoped deletion declarations and calls =='
rg -n -C 5 --glob '*.java' \
  '\bdeleteJob\s*\(\s*[^,\)]*,\s*(?:context|[A-Za-z][A-Za-z0-9_]*Context)\s*\)|\bdeleteJob\s*\(\s*[^,\)]*,\s*TenantContext\b' \
  src

echo '== Administrative-controller construction sites =='
rg -n -C 4 --glob '*.java' \
  '\bnew\s+AdminController\s*\(' \
  src

echo '== Tenant authorization and exception mapping =='
for f in \
  "$(fd -t f 'TenantAccessService.java' src/main/java -1)" \
  "$(fd -t f 'TenantContext.java' src/main/java -1)" \
  "$(fd -t f 'ApiExceptionHandler.java' src/main/java -1)" \
  "$(fd -t f 'DocumentConversionService.java' src/main/java -1)"
do
  if [ -n "$f" ]; then
    echo "----- $f -----"
    cat -n "$f"
  fi
done

Length of output: 52492


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Repository contracts and implementations =='
fd -t f -e java . src/main/java | rg '/.*(?:ConversionJobRepository|.*ConversionJob.*Repository|.*Job.*Store)\.java$'
rg -n -C 8 --glob '*.java' \
  '\b(findByTenantAndId|deleteById|belongsToTenant)\s*\(' \
  src/main/java

echo '== Service contract =='
service_file="$(fd -t f 'DocumentConversionService.java' src/main/java -1)"
cat -n "$service_file"

echo '== Deletion-related tests =='
rg -n -C 6 --glob '*Test.java' \
  '\b(deleteJob|findByTenantAndId|belongsToTenant|tenant.*delete|delete.*tenant)\b' \
  src/test

echo '== Spring application and controller bean wiring =='
fd -t f -e java . src/main/java | rg '/.*(?:Application|Configuration|Config)\.java$' | while IFS= read -r f; do
  echo "----- $f -----"
  ast-grep outline "$f" --items all --type class,method
  rg -n -C 4 --glob '*.java' \
    '\bAdminController\s*\(|\bTenantAccessService\s*\(' "$f" || true
done

Length of output: 78980


@seonghobae Reviewed commit 3187b9b5b1a2c9a8c9f9b27fbaee5808721daaef against 961f44a19c4ced87f8062b3ab9842a9a08d05235.

I found no blocking issues in the three-file change.

AdminController.deleteJob now requires JOB_DELETE, passes the verified TenantContext to the tenant-aware service method, and returns 404 for missing or foreign jobs. The service verifies tenant ownership before deletion. The added tests cover missing claims, missing permission, tenant-scoped delegation, concealed foreign jobs, and successful owned-job deletion.

The PR is mergeable. The reported CI, security, SAST, fuzz, and review checks are successful. No unresolved review threads exist.

This review is read-only. I did not modify the branch or approve the pull request. Independent approval remains required.

You are interacting with an AI system.

Copy link
Copy Markdown
Collaborator Author

Superseded by #440.

The canonical auth/RBAC contract defines tenant_admin with tenant:configure; it does not define job:delete as the administrative authority for this route. #440 applies that documented authority to every AdminController operation, preserves tenant context for list/delete/retry, conceals foreign jobs as 404, and includes the same missing-claim, missing-permission, tenant-scoped deletion, and unscoped-mutation exclusion regressions covered here.

Keeping this narrower branch open would create a competing permission contract and overlapping AdminController writer. Continue exact-head review and protected merge on #440.

@seonghobae seonghobae closed this Aug 15, 2026
auto-merge was automatically disabled August 15, 2026 09:16

Pull request was closed

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