fix(security): fail closed on missing conversion-job authority

[seonghobae]

Objective

Make the explicit tenant-aware ConversionJob constructor fail closed rather than silently replacing absent authority with buyer-demo identities.

Exact current-head evidence

• protected base: eb0a32e87e7470469c8fa0f8c67d9583654bef57;
• exact head: b38f1207d2ed03f72250e3a1eae046a83b5bb0ff;
• changed paths: ConversionJob.java and focused authority regressions only;
• CI, Security Scan, SAST Semgrep, and fuzz: terminal-success;
• mergeable: true;
• unresolved inline findings: zero.

Authority contract

The constructor whose signature receives tenant and subject authority rejects null, blank, NUL/control-only, and unusable values with controlled errors. Valid values are normalized once. Explicit legacy/demo convenience constructors remain separate compatibility surfaces.

Scope boundary

Conversion-job authority model only. This does not change viewer UI, repositories, lifecycle state, credentials, OIDC, Office conversion, or the legacy demo constructors.

Merge gate

Keep this exact head unchanged. Auto-merge may act only after live required checks remain terminal-success, zero valid unresolved findings remain, and a qualifying independent non-author approval is attached to this exact head.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e92e0c68-03fb-4d5e-a332-f0b9d4853c43

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

ConversionJob은 tenantId와 subjectId를 필수 권한 메타데이터로 검증합니다. null 문자 제거 후 공백인 값도 거부합니다. 누락된 권한 정보를 Demo 값으로 대체하던 동작을 제거하고 관련 테스트를 추가했습니다.

Changes

권한 메타데이터 검증

Layer / File(s)	Summary
필수 권한 검증 구현 src/main/java/com/clearfolio/viewer/model/ConversionJob.java	생성자는 tenantId와 subjectId를 requireAuthority로 검증합니다. null 문자 제거와 양끝 공백 제거 후 값이 비어 있으면 IllegalArgumentException을 발생시킵니다.
권한 검증 테스트 src/test/java/com/clearfolio/viewer/model/ConversionJobTenantAuthorityTest.java, src/test/java/com/clearfolio/viewer/model/ConversionJobTest.java	null 또는 정제 후 공백인 권한 값의 예외 유형과 메시지를 검증합니다. Demo 대체 동작이 제거된 사례도 검증합니다.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• ContextualWisdomLab/clearfolio#341: 다른 코드 경로에서 테넌트 권한 검증을 수정합니다.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% 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
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	제목은 누락된 변환 작업 권한 정보를 기본값으로 대체하지 않고 안전하게 실패하도록 변경한 핵심 내용을 정확히 요약합니다.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/explicit-tenant-job-authority-main-20260811

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[seonghobae]

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

Review exact head b38f1207d2ed03f72250e3a1eae046a83b5bb0ff only. Exact-head CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; unresolved inline findings are zero. Validate fail-closed explicit tenant/subject construction while preserving separately named legacy/demo convenience constructors.

Review read-only; preserve independent reviewer identity. Do not mutate the branch, synthesize author approval, weaken protection, or merge.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

requested_head='b38f1207d2ed03f72250e3a1eae046a83b5bb0ff'
repo='ContextualWisdomLab/clearfolio'
pr='348'

echo '== Local commit identity =='
git rev-parse HEAD
git status --short
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$requested_head"
if [ "$(git rev-parse HEAD)" = "$requested_head" ]; then
  echo 'HEAD_MATCHES_REQUESTED=true'
else
  echo 'HEAD_MATCHES_REQUESTED=false'
fi

echo '== Pull request metadata and changed files =='
gh pr view "$pr" --repo "$repo" --json number,state,isDraft,headRefOid,baseRefOid,url \
  --jq '{number,state,isDraft,headRefOid,baseRefOid,url}'
gh pr diff "$pr" --repo "$repo" --name-only

echo '== Exact-head diff =='
base="$(gh pr view "$pr" --repo "$repo" --json baseRefOid --jq .baseRefOid)"
git diff --find-renames --find-copies "$base" "$requested_head" -- \
  src/main/java/com/clearfolio/viewer/model/ConversionJob.java \
  src/test/java/com/clearfolio/viewer/model/ConversionJobTenantAuthorityTest.java \
  src/test/java/com/clearfolio/viewer/model/ConversionJobTest.java

echo '== ConversionJob structure =='
ast-grep outline src/main/java/com/clearfolio/viewer/model/ConversionJob.java --items all --view expanded
ast-grep outline src/test/java/com/clearfolio/viewer/model/ConversionJobTenantAuthorityTest.java --items all --view expanded
ast-grep outline src/test/java/com/clearfolio/viewer/model/ConversionJobTest.java --items all --view expanded

echo '== Constructor call sites and demo identifiers =='
rg -n -C 3 --glob '*.java' 'new\s+ConversionJob\s*\(' src
rg -n -C 3 --glob '*.java' '(buyer-demo|requireAuthority|tenantId|subjectId)' \
  src/main/java/com/clearfolio/viewer/model/ConversionJob.java \
  src/test/java/com/clearfolio/viewer/model/ConversionJobTenantAuthorityTest.java \
  src/test/java/com/clearfolio/viewer/model/ConversionJobTest.java

echo '== Exact-head check runs =='
gh api "repos/$repo/commits/$requested_head/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name,.status,.conclusion,.details_url] | `@tsv`'

echo '== Pull request reviews and review threads =='
gh pr view "$pr" --repo "$repo" --json reviews \
  --jq '.reviews[] | [.author.login,.state,.commit.oid,.submittedAt] | `@tsv`'
gh api graphql -F owner='ContextualWisdomLab' -F name='clearfolio' -F number="$pr" -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: 20) {
            nodes { author { login } body path line originalLine commit { oid } }
          }
        }
      }
    }
  }
}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] |
  select(.isResolved == false) |
  {isOutdated, comments: [.comments.nodes[] | {author: .author.login, path, line, originalLine, oid: .commit.oid, body}]}'

Length of output: 113155

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== ConversionJob constructors and authority helpers =='
sed -n '1,155p' src/main/java/com/clearfolio/viewer/model/ConversionJob.java

echo '== Tenant comparison behavior =='
sed -n '176,190p' src/main/java/com/clearfolio/viewer/model/ConversionJob.java

echo '== Production construction path =='
sed -n '150,215p' src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java

echo '== Production explicit constructor calls =='
ast-grep run \
  --pattern $'new ConversionJob($JOB_ID, $TENANT_ID, $SUBJECT_ID, $FILE_NAME, $CONTENT_TYPE, $HASH, $SIZE, $MAX_ATTEMPTS)' \
  --lang java src/main/java

echo '== Legacy/demo constructor and explicit constructor declarations =='
ast-grep run \
  --pattern $'public ConversionJob($$$PARAMS) { $$$BODY }' \
  --lang java src/main/java/com/clearfolio/viewer/model/ConversionJob.java

echo '== Authority fallback references in production code =='
rg -n -C 2 --glob '*.java' '(DEFAULT_TENANT_ID|DEFAULT_SUBJECT_ID|buyer-demo|normalizeOrDefault\s*\()' src/main/java

Length of output: 17370

---

@seonghobae Review complete for exact head b38f1207d2ed03f72250e3a1eae046a83b5bb0ff.

No actionable findings.

• The explicit ConversionJob constructor now rejects null and blank-after-NUL-removal tenantId and subjectId.
• The controlled errors identify the invalid authority field.
• The legacy convenience constructors still explicitly supply the demo tenant and subject identities.
• The production tenant-aware submission path passes the TenantContext tenant and subject claims to the explicit constructor.
• The regression tests cover null and blank-after-sanitization claims for both authority fields.
• Maven tests, coverage, security scans, Semgrep SAST, and listed fuzz checks completed successfully on the requested commit.
• No unresolved inline review threads were returned.

This was a read-only review. I did not mutate the branch, approve, or merge.

⚠️ Action not completed

No files to review.

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.