Skip to content

fix(security): reject missing tenant authority at submission boundary - #346

Closed
seonghobae wants to merge 5 commits into
mainfrom
fix/null-tenant-submit-fail-closed-20260810
Closed

fix(security): reject missing tenant authority at submission boundary#346
seonghobae wants to merge 5 commits into
mainfrom
fix/null-tenant-submit-fail-closed-20260810

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Objective

Advance issue #317 with one path-local production security boundary: the tenant-aware service entrypoint must fail closed when authenticated tenant context is absent instead of silently manufacturing buyer-demo authority and continuing validation, deduplication, persistence, artifact work, and enqueue.

Fresh exact state — 2026-08-10

  • exact source head: d4a59a72b485b76c7870770ca69c6963bc8346c1;
  • independently resolved protected main: 55d7ae8647208e301f282350f076eeddaba61d11;
  • mergeable clean current-base slice, three changed files and three commits;
  • exact-head CI 31404369157: success;
  • exact-head Security Scan 31404368715: success;
  • exact-head SAST Semgrep 31404369100: success;
  • exact-head fuzz 31404369115: success;
  • authoritative Maven exact checkout was d4a59a72b485b76c7870770ca69c6963bc8346c1; 497 tests ran with 0 failures, 0 errors and 0 skips; all JaCoCo coverage checks and public Javadocs passed;
  • no formal reviews or unresolved review threads at the latest direct refetch;
  • automated check/model evidence is not qualifying independent approval.

Test-first evidence

RED

Test-only head d6a6729d21fd2c0adc876b2b89bb5991c4d086b0 added a focused regression around the three-argument tenant-aware submit(...) service boundary before production changes. Exact-head CI 31403123801 checked out that exact SHA and failed one of 497 tests. Instead of the required controlled IllegalArgumentException("tenant context is required"), protected-main behavior continued through the null-context demo fallback and eventually produced a NullPointerException when the mocked repository returned no canonical result. That proved the service crossed validation/repository boundaries after authority was absent. Security Scan, Semgrep and fuzz were already GREEN on the RED head.

GREEN

The tenant-aware three-argument submit(...) now rejects a null TenantContext immediately, before policy validation, hashing, deduplication, persistence, PDF artifact seeding or enqueue. The two-argument demo convenience overload remains explicit and continues to construct its own demo TenantContext; this slice therefore removes only the unsafe implicit null-to-demo escalation from the production tenant-aware boundary.

Focused GREEN tests prove:

  • null tenant authority yields exactly IllegalArgumentException("tenant context is required");
  • validation service, repository, worker and artifact store have zero interactions on that denial path;
  • no job is stored and no worker enqueue occurs;
  • the pre-existing service regression is aligned with the fail-closed contract rather than asserting the removed fallback.

The unchanged exact head is GREEN through the full Maven lifecycle, synthetic-main merge compatibility, security scan, Semgrep and fuzz.

Scope boundary

This does not remove the legacy two-argument demo convenience overload, redesign browser/session bootstrap, modify viewer.js, demo.js or dom-utils.js, implement OIDC/JWT federation, or touch active Office adapter #306. It is a narrow fail-closed prerequisite for issue #317's broader production workspace authority work and does not claim that demo authority has been fully removed.

Merge gate

Move to Ready for ordinary independent review. Keep this exact head unchanged. Merge or auto-merge may complete only after live protection still sees all applicable required gates passing, zero valid unresolved findings, and any required qualifying independent non-author approval. Automated evidence is not approval.

Summary by CodeRabbit

  • 버그 수정
    • 테넌트 권한 정보 없이 문서 제출을 시도하면 명확한 오류가 표시됩니다.
    • 유효한 테넌트 및 주체 정보가 있는 경우에만 문서 변환 작업이 생성됩니다.
    • 잘못된 제출 시 저장이나 작업 처리가 진행되지 않도록 개선했습니다.

@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: a92d2c58-6c54-4f0a-81b8-bbce65fb2a10

📥 Commits

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

📒 Files selected for processing (3)
  • src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java
  • src/test/java/com/clearfolio/viewer/service/DefaultDocumentConversionServiceTenantAuthorityTest.java
  • src/test/java/com/clearfolio/viewer/service/DefaultDocumentConversionServiceTest.java

📝 Walkthrough

Walkthrough

submittenantContext가 없으면 IllegalArgumentException을 발생시킵니다. 유효한 컨텍스트의 테넌트와 주체 식별자를 사용해 작업을 생성합니다. 관련 테스트는 예외와 작업 미생성 동작을 검증합니다.

Changes

테넌트 권한 검증

Layer / File(s) Summary
제출 시 테넌트 컨텍스트 검증
src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java
tenantContext가 없으면 "tenant context is required" 예외를 발생시킵니다. 유효한 컨텍스트의 테넌트 및 주체 식별자를 ConversionJob 생성에 사용합니다.
실패 동작 검증
src/test/java/com/clearfolio/viewer/service/DefaultDocumentConversionServiceTenantAuthorityTest.java, src/test/java/com/clearfolio/viewer/service/DefaultDocumentConversionServiceTest.java
테넌트 컨텍스트가 없을 때 예외가 발생하는지 검증합니다. 문서 검증, 저장소 저장, 작업 큐 등록, 아티팩트 저장소 상호작용이 없는지 확인합니다.

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

Mergeability Score: ⚪ Minimal · up to b43a9

This change rejects submissions without tenant authority before any validation or downstream processing, preventing unintended demo-authority use. No actionable merge-blocking risk remains beyond normal checks and independent review.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 테넌트 권한이 없을 때 제출을 거부하는 핵심 보안 변경을 정확하고 간결하게 설명합니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/null-tenant-submit-fail-closed-20260810

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 10, 2026 15:39
@seonghobae
seonghobae enabled auto-merge (squash) August 10, 2026 15:40

Copy link
Copy Markdown
Collaborator Author

Superseded by current-main replacement #438. The replacement preserves this PR's three-file fail-closed tenant-aware submission semantic delta on protected main 961f44a19c4ced87f8062b3ab9842a9a08d05235, has fresh exact-head CI/Security/SAST/fuzz evidence, is Ready, and has auto-merge enabled subject to ordinary independent approval and branch protection. Closing this stale-base duplicate prevents parallel review and merge ambiguity; no predecessor evidence transfers.

@seonghobae seonghobae closed this Aug 14, 2026
auto-merge was automatically disabled August 14, 2026 07:54

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