Skip to content

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

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

fix(security): reject missing tenant authority at submission boundary#346
seonghobae wants to merge 3 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.

@coderabbitai

coderabbitai Bot commented Aug 10, 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: 6 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: a7f8ecfb-fd95-4a84-9761-eda4a21d6c4d

📥 Commits

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

📒 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

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
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