Skip to content

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

Open
seonghobae wants to merge 4 commits into
mainfrom
fix/explicit-tenant-job-authority-main-20260811
Open

fix(security): fail closed on missing conversion-job authority#348
seonghobae wants to merge 4 commits into
mainfrom
fix/explicit-tenant-job-authority-main-20260811

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Objective

Advance #317 with a path-disjoint production security slice: the explicit tenant-aware ConversionJob constructor must not silently replace absent tenant/subject authority with buyer-demo identities.

Test-first state

This Draft is intentionally at a RED test-only head. ConversionJobTenantAuthorityTest requires null or blank-after-NUL-removal tenant/subject claims to fail closed with controlled errors. Protected main currently falls back to demo authority, so CI is expected to fail until the narrow model boundary is corrected.

Scope

This slice touches only the conversion-job authority model and its focused regression. It does not edit active viewer.js, demo.js, dom-utils.js, ViewerUiController, repository/lifecycle, credential, OIDC, Office-conversion, or canonical-document writer paths. Legacy demo convenience constructors remain explicit development compatibility surfaces; this PR changes only the constructor whose signature claims that tenant and subject authority were supplied.

Acceptance

RED -> smallest production fix -> exact-head mvn verify/100% owned coverage/public Javadocs -> CI/Security Scan/SAST/fuzz -> review/thread refetch. Keep Draft until GREEN exact-head evidence exists; independent write-authorized approval remains a separate merge gate.

Summary by CodeRabbit

  • 버그 수정
    • 변환 작업 생성 시 테넌트 및 주체 식별자의 필수 입력을 검증합니다.
    • 식별자가 null, 빈 값 또는 제어 문자 제거 후 공백인 경우 오류를 표시합니다.
    • 식별자에 포함된 null 문자와 앞뒤 공백을 정리해 저장합니다.
    • 유효하지 않거나 누락된 권한 정보를 임의의 기본 Demo 값으로 대체하지 않습니다.
    • 권한 검증 오류 메시지를 명확하게 제공해 잘못된 설정을 쉽게 확인할 수 있습니다.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1f977a86-f4a5-4188-a62b-b2bbf44067f9

📥 Commits

Reviewing files that changed from the base of the PR and between 55d7ae8 and 5b5b453.

📒 Files selected for processing (3)
  • 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

📝 Walkthrough

Walkthrough

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

Changes

권한 메타데이터 검증

Layer / File(s) Summary
필수 권한 검증 구현
src/main/java/com/clearfolio/viewer/model/ConversionJob.java
생성자는 tenantIdsubjectIdrequireAuthority로 검증합니다. 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

🚥 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 누락된 conversion-job 권한 정보를 기본값으로 대체하지 않고 실패 처리하는 핵심 보안 변경을 정확히 요약합니다.
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.
✨ 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

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

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