fix(security): enforce tenant context authority invariant

[seonghobae]

Objective

Make the internal tenant-authority value object fail closed when callers directly construct it without usable tenant or subject identity. Header parsing already rejects absent claims, but every service boundary must preserve the same invariant.

Exact current-head evidence

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

Authority contract

Direct construction rejects null, blank, NUL-only, and surrounding-whitespace/control-normalized tenant or subject identities with controlled field-specific errors. Valid authority is normalized once and permission semantics remain unchanged.

Scope boundary

TenantContext constructor invariant only. This does not change signed-header verification, OIDC/JWT federation, permission vocabulary, controllers, persistence, or demo-only entrypoints.

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. Any head/base movement requires complete exact-head revalidation.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 3 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: b00d4ee2-a314-4c2f-bd96-093ca5ff502b

📥 Commits

Reviewing files that changed from the base of the PR and between eb0a32e and 45c35e3.

📒 Files selected for processing (2)

• src/main/java/com/clearfolio/viewer/auth/TenantContext.java
• src/test/java/com/clearfolio/viewer/auth/TenantContextAuthorityInvariantTest.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

• 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 af82459f6e3428100ff88f9cd0dc194c59360ecd only. Exact-head CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; unresolved inline findings are zero. Validate fail-closed direct construction for absent/control-only tenant or subject authority while preserving valid normalization and permission semantics.

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

echo '== checked-out commit =='
git rev-parse HEAD
echo '== requested commit exists =='
git rev-parse af82459f6e3428100ff88f9cd0dc194c59360ecd^{commit}
echo '== diff paths against stated base =='
git diff --name-status eb0a32e87e7470469c8fa0f8c67d9583654bef57 af82459f6e3428100ff88f9cd0dc194c59360ecd
echo '== diff =='
git diff --find-renames --find-copies --unified=80 eb0a32e87e7470469c8fa0f8c67d9583654bef57 af82459f6e3428100ff88f9cd0dc194c59360ecd

echo '== candidate paths =='
fd -t f 'TenantContext(\.java|Test\.java)$' .

echo '== TenantContext outline =='
tenant_file="$(fd -t f '^TenantContext\.java$' . | head -n 1)"
test_file="$(fd -t f '^TenantContextTest\.java$' . | head -n 1)"
printf 'source: %s\ntest: %s\n' "$tenant_file" "$test_file"
ast-grep outline "$tenant_file" --items all --view expanded
if [ -n "$test_file" ]; then
  ast-grep outline "$test_file" --items all --view expanded
fi

echo '== TenantContext construction and permission call contexts =='
rg -n --type java -C 4 \
  'new\s+TenantContext\s*\(|\b(hasPermission|isAuthorized|canAccess|normalize|validate)\s*\(' \
  . -g '!**/target/**'

echo '== GitHub PR metadata =='
gh pr view 356 --repo ContextualWisdomLab/clearfolio \
  --json number,headRefOid,baseRefOid,mergeStateStatus,reviewDecision,statusCheckRollup,reviews \
  --jq '{number,headRefOid,baseRefOid,mergeStateStatus,reviewDecision,statusCheckRollup:[.statusCheckRollup[]|{name,status,conclusion}],reviews:[.reviews[]|{author:.author.login,state,commit:.commit.oid}]}'

Length of output: 48375

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

tenant_file='src/main/java/com/clearfolio/viewer/auth/TenantContext.java'
authority_test='src/test/java/com/clearfolio/viewer/auth/TenantContextAuthorityInvariantTest.java'
existing_test='src/test/java/com/clearfolio/viewer/auth/TenantContextTest.java'

echo '== TenantContext with line numbers =='
nl -ba "$tenant_file" | sed -n '55,155p'

echo '== focused authority regression with line numbers =='
nl -ba "$authority_test"

echo '== existing normalization and permission regression =='
nl -ba "$existing_test" | sed -n '1,115p'

echo '== character-policy references =='
rg -n --type java -C 3 \
  '(\\u0000|CONTROL|control.character|isISOControl|isControl|strip\(\)|sanitize\()' \
  src/main/java src/test/java

echo '== direct constructor production call sites =='
rg -n --type java -C 5 'new\s+TenantContext\s*\(' src/main/java

echo '== latest check detail for failed strix check =='
gh pr checks 356 --repo ContextualWisdomLab/clearfolio --json name,state,link,workflow,bucket \
  --jq '.[] | select(.name == "strix")'

Length of output: 1440

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

tenant_file='src/main/java/com/clearfolio/viewer/auth/TenantContext.java'
authority_test='src/test/java/com/clearfolio/viewer/auth/TenantContextAuthorityInvariantTest.java'
existing_test='src/test/java/com/clearfolio/viewer/auth/TenantContextTest.java'

echo '== TenantContext with line numbers =='
cat -n "$tenant_file" | sed -n '55,155p'

echo '== focused authority regression with line numbers =='
cat -n "$authority_test"

echo '== existing normalization and permission regression =='
cat -n "$existing_test" | sed -n '1,115p'

echo '== character-policy references =='
rg -n --type java -C 3 \
  '(\\u0000|CONTROL|control.character|isISOControl|isControl|strip\(\)|sanitize\()' \
  src/main/java src/test/java

echo '== direct constructor production call sites =='
rg -n --type java -C 5 'new\s+TenantContext\s*\(' src/main/java

echo '== latest check detail for failed strix check =='
gh pr checks 356 --repo ContextualWisdomLab/clearfolio --json name,state,link,workflow,bucket \
  --jq '.[] | select(.name == "strix")'

Length of output: 66663

---

@seonghobae Review complete for exact head af82459f6e3428100ff88f9cd0dc194c59360ecd.

Blocking finding — TenantContext accepts non-NUL control-only authority.
TenantContext.sanitize at src/main/java/com/clearfolio/viewer/auth/TenantContext.java, Line 146 removes only U+0000. String.strip() at Line 147 does not remove every control character. For example, direct construction with "\u0001" as tenantId or subjectId succeeds. This creates a context with no usable tenant or subject identity. It violates the requested fail-closed invariant for control-only authority.

Reject or normalize all control characters before the empty-value check. Add constructor regression cases for non-NUL control-only tenant and subject values. Preserve the current whitespace normalization and permission behavior.

I also verified the stated base and head SHA values. The live strix check is currently FAILURE for this exact head. Therefore, the exact-head checks are not all terminal-success, and this PR does not meet the stated merge gate.

⚠️ Action not completed

Review rate limited.

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.