fix(security): require signed tenant claims on admin endpoints - #266
fix(security): require signed tenant claims on admin endpoints#266seonghobae wants to merge 47 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
@coderabbitai review Review clean stacked exact head |
|
I will focus on signed tenant claims, least-privilege permissions, tenant isolation, mutation-boundary authorization, audit pseudonymization, config-tree secret loading, API and deployment contracts, and tests. I will exclude superseded
|
|
@opencode-agent @cwl-noema-review Independently review clean stacked exact head |
|
@opencode-agent Fix the valid tenant-mutation atomicity gap on exact stacked head The current Add explicit tenant-scoped mutation contracts at the repository/state-store boundary and implement them atomically in Add deterministic tests that prove: cross-tenant and missing targets never call or apply unscoped mutation; an ownership change/replacement between a preliminary observation and mutation cannot delete or retry a different-tenant job; accepted retry records the actor fingerprint and enqueues exactly once; failed/not-eligible paths do not enqueue; tenant-scoped delete removes the content-hash index only for the actually deleted owned job. Add beginner-readable public Javadocs and update the administrative authorization ADR so its atomicity claim exactly matches the implementation. Run |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Manual exact-head review found one valid tenant-isolation layering gap. AdminController.getAllJobs() calls the global DocumentConversionService.getAllJobs() and only then filters ConversionJob objects by tenant in the web layer. Although the response is currently filtered, this materializes every tenant's jobs above the persistence/service authorization boundary and makes a future durable adapter prone to accidental cross-tenant disclosure, excessive reads, and audit ambiguity. Keep this draft stacked on #165 and address the issue test-first: add an explicit tenant-scoped repository/service query contract, make the default production service delegate the authenticated tenant identifier to that boundary, and make AdminController consume only the tenant-scoped result before applying the optional dead-letter filter. Add tests proving the controller never invokes the global list path, the service passes only the authenticated tenant ID, and the in-memory repository cannot return other tenants' jobs. Preserve getAllJobs() only where backward compatibility or non-administrative internals require it; do not weaken signed-claim, concealment, audit, coverage, or parent privacy behavior. Run full local mvn -B --no-transfer-progress verify and report the exact new head.
|
@opencode-agent Implement the exact-head tenant-scoped administrative list finding in review |
|
@coderabbitai review Review stacked exact head |
|
I will verify that I will keep
|
|
Manual exact-head review found one still-valid HIGH authorization blocker on
Keep this PR draft. After parent #165 is integrated, fix test-first by adding fail-closed tenant-predicate mutation contracts to the repository/state-store boundary, implementing them atomically in the in-memory adapter, and making durable adapters explicitly implement or reject the operation. Add deterministic concurrency/replacement tests proving that a same-identifier cross-tenant replacement cannot be deleted, retried, transitioned, or enqueued. Re-run the exact restacked head through full CI, zero-missed line/branch coverage, security scans, fuzzing, Strix/OpenCode/Noema, and independent approval before merge. |
|
@coderabbitai review @opencode-agent @cwl-noema-review Please review exact current head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Exact HEAD =='
git rev-parse HEAD
git status --short
git show -s --format='%H%n%P%n%s' HEAD
printf '%s\n' '== Candidate files =='
fd -t f -a . | rg '(DocumentConversionService|ConversionJobRepository|ConversionJobStateStore|InMemoryConversionJobRepository|TenantScopedServiceDefaultsTest|AdminController|CHANGELOG|AGENTS|ADR|Authorization|Security)'
printf '%s\n' '== Symbol outlines =='
for f in $(fd -t f -a . | rg '(DocumentConversionService|DefaultDocumentConversionService|ConversionJobRepository|ConversionJobStateStore|InMemoryConversionJobRepository|TenantScopedServiceDefaultsTest|AdminController)' | head -40); do
echo "--- $f"
ast-grep outline "$f" --items all --view compact 2>/dev/null || true
done
printf '%s\n' '== Targeted symbol references =='
rg -n -C 4 \
'(deleteJob\s*\([^)]*TenantContext|retryDeadLettered\s*\([^)]*TenantContext|getJobsForTenant\s*\(|findByTenantAndId\s*\(|findAllByTenantId\s*\(|deleteByTenant|retryDeadLetteredByTenant|deleteById\s*\(|retryDeadLettered\s*\(|getJob\s*\()' \
. -g '!**/target/**' -g '!**/.git/**'
printf '%s\n' '== Relevant head commits =='
git log --oneline --decorate -12
printf '%s\n' '== Changed files since likely stack parent =='
git diff --name-status 5261356ac34e6545bce947ba0bcf2b1ce9f9be67..HEAD 2>/dev/null || \
git diff --name-status HEAD~12..HEAD
printf '%s\n' '== Changelog and security-document references =='
rg -n -C 3 \
'(tenant-aware|tenant-scoped|fail-closed|administrative|atomic|HMAC|pseudonym|delete|retry)' \
CHANGELOG.md AGENTS.md . -g '*.md' -g '!**/target/**' -g '!**/.git/**'Length of output: 200290 🏁 Script executed: #!/bin/bash
set -euo pipefail
find_one() {
local name="$1"
fd -t f -a -E target "$name" . | head -1
}
service_iface="$(find_one 'DocumentConversionService.java')"
service_impl="$(find_one 'DefaultDocumentConversionService.java')"
repo_iface="$(find_one 'ConversionJobRepository.java')"
state_iface="$(find_one 'ConversionJobStateStore.java')"
repo_impl="$(find_one 'InMemoryConversionJobRepository.java')"
defaults_test="$(find_one 'TenantScopedServiceDefaultsTest.java')"
admin_controller="$(find_one 'AdminController.java')"
adr="docs/security/2026-08-05-administrative-authorization.md"
printf '== paths ==\n%s\n' \
"$service_iface" "$service_impl" "$repo_iface" "$state_iface" "$repo_impl" \
"$defaults_test" "$admin_controller" "$adr"
printf '%s\n' '== Service interface: tenant-aware methods and defaults =='
rg -n -C 12 \
'(default .*deleteJob|default .*retryDeadLettered|deleteJob\s*\(|retryDeadLettered\s*\(|getJob\s*\()' \
"$service_iface"
printf '%s\n' '== Default service: tenant-aware overrides and direct calls =='
rg -n -C 14 \
'(deleteJob\s*\([^)]*TenantContext|retryDeadLettered\s*\([^)]*TenantContext|findByTenantAndId|deleteById|retryDeadLettered\s*\(|getJob\s*\()' \
"$service_impl"
printf '%s\n' '== Repository and state-store contracts =='
rg -n -C 10 \
'(findByTenantAndId|delete|retryDeadLettered|TenantContext|tenant)' \
"$repo_iface" "$state_iface" "$repo_impl"
printf '%s\n' '== Regression test =='
cat -n "$defaults_test"
printf '%s\n' '== Admin controller list/mutation calls =='
rg -n -C 8 \
'(getAllJobs|getJobsForTenant|deleteJob\s*\(|retryDeadLettered\s*\()' \
"$admin_controller"
printf '%s\n' '== Administrative ADR relevant section =='
sed -n '19,66p' "$adr"
printf '%s\n' '== Exact final commits =='
git show --stat --oneline 8fbf2d118b94aff9f0a4657aaf098217ffa4fca9
git diff --check 8fbf2d118b94aff9f0a4657aaf098217ffa4fca9^ 8fbf2d118b94aff9f0a4657aaf098217ffa4fca9
git show --format=fuller --find-renames --find-copies --stat 5bf134e606c009584a8fc78a7a45510ad41e65fc
git diff --check 5bf134e606c009584a8fc78a7a45510ad41e65fc^ 5bf134e606c009584a8fc78a7a45510ad41e65fcLength of output: 54624 🏁 Script executed: #!/bin/bash
set -euo pipefail
iface="$(fd -t f -a -E target -p 'DocumentConversionService.java' . | rg '/DocumentConversionService\.java$' | head -1)"
impl="$(fd -t f -a -E target -p 'DefaultDocumentConversionService.java' . | rg '/DefaultDocumentConversionService\.java$' | head -1)"
repo="$(fd -t f -a -E target -p 'ConversionJobRepository.java' . | rg '/ConversionJobRepository\.java$' | head -1)"
state="$(fd -t f -a -E target -p 'ConversionJobStateStore.java' . | rg '/ConversionJobStateStore\.java$' | head -1)"
inmemory="$(fd -t f -a -E target -p 'InMemoryConversionJobRepository.java' . | rg '/InMemoryConversionJobRepository\.java$' | head -1)"
printf '%s\n' '== Exact HEAD and interface =='
git rev-parse HEAD
cat -n "$iface" | sed -n '1,230p'
printf '%s\n' '== Concrete tenant-aware mutation paths =='
cat -n "$impl" | sed -n '214,310p'
printf '%s\n' '== Available mutation contracts =='
cat -n "$repo" | sed -n '60,135p'
cat -n "$state" | sed -n '1,80p'
cat -n "$inmemory" | sed -n '138,152p;232,253p'
printf '%s\n' '== Tenant-aware mutation call graph, excluding tests =='
rg -n --type java -C 2 \
'(deleteJob\s*\([^,]+,\s*(tenantContext|context)|retryDeadLettered\s*\([^,]+,\s*(tenantContext|context)|findByTenantAndId\s*\(|deleteById\s*\(|stateStore\.retryDeadLettered\s*\()' \
src/main/java
printf '%s\n' '== Stack file summary and key-file diffs from declared parent =='
git diff --stat 5261356ac34e6545bce947ba0bcf2b1ce9f9be67..HEAD
git diff --name-status 5261356ac34e6545bce947ba0bcf2b1ce9f9be67..HEAD
git diff --numstat 5261356ac34e6545bce947ba0bcf2b1ce9f9be67..HEAD -- \
src/main/java/com/clearfolio/viewer/service/DocumentConversionService.java \
src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobStateStore.java \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/test/java/com/clearfolio/viewer/service/TenantScopedServiceDefaultsTest.java \
docs/security/2026-08-05-administrative-authorization.md \
CHANGELOG.mdLength of output: 27774
HIGH — The concrete tenant-aware mutations are still not atomic.
However,
A replacement of the same Add explicit tenant-scoped mutation contracts. Implement them atomically in The compatibility-default regression test, tenant-scoped list boundary, and fail-closed modular intent are valid. I found no unrelated parent privacy or PDF.js reversion in the inspected stack file changes. I did not approve this draft. Missing GitHub Actions evidence is not passing evidence. Keep the draft stacked until You are interacting with an AI system. |
|
@coderabbitai @opencode-agent @cwl-noema-review Review exact stacked head Verify the concurrency test is deterministic and non-flaky; the lock is reentrant for blank-hash find-or-store; no tenant, deduplication, lifecycle, retry, or standalone-adapter contract regresses; public Javadocs and 100% production line/branch coverage remain intact; and no raw identifiers enter audit evidence. Run |
|
Exact current stacked head
A separate end-to-end document-lifecycle gap remains and is specified in #263 rather than being hidden by this narrower persistence claim. After Keep #266 draft and stacked on #165. Its atomicity claim is limited to tenant-scoped metadata/state mutation and secondary-index consistency. Implement durable, truthful, idempotent deletion and generation-bound dispatch in #263 after #264/#266 integrate. @opencode-agent @cwl-noema-review @coderabbitai review exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='310b2ee67dff58ef44b4c10c8b7b07ce878e3dc0'
actual="$(git rev-parse HEAD)"
printf '== exact head ==\nexpected=%s\nactual=%s\n' "$expected" "$actual"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' HEAD
printf '\n== commit range since red test ==\n'
git log --oneline b47464f3c1a2c75821dba121e090f27b089e178d^..HEAD
printf '\n== changed paths from declared stack parent ==\n'
git diff --name-status 5261356ac34e6545bce947ba0bcf2b1ce9f9be67..HEAD
printf '\n== candidate files ==\n'
fd -t f -a -E target . | rg '(InMemoryConversionJobRepository|ConversionJobRepository|ConversionJobStateStore|DefaultDocumentConversionService|DocumentConversionService|Atomic|Concurrency|Stale|Collision|Audit|AdministrativeAudit|CHANGELOG|administrative-authorization)' | sort
printf '\n== production outlines ==\n'
for f in \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobStateStore.java \
src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java \
src/main/java/com/clearfolio/viewer/audit/AdministrativeAuditLogger.java \
src/main/java/com/clearfolio/viewer/security/AuditPseudonymizer.java
do
if [ -f "$f" ]; then
echo "--- $f"
ast-grep outline "$f" --items all --view compact || true
fi
done
printf '\n== shared-lock/index/mutation implementation ==\n'
rg -n -C 12 --type java \
'(synchronized|Lock|Reentrant|primary|contentHash|tenantContent|findOrStore|findAllByTenantId|deleteByTenantAndId|deleteById|retryDeadLetteredForTenant|retryDeadLettered)' \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobStateStore.java \
src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java
printf '\n== exact concurrency and relevant regression tests ==\n'
fd -t f -a -E target . | rg '(Atomic|Concurrency|Stale|Collision|TenantScope|InMemoryConversionJobRepository).*Test\.java$' | sort | while read -r f; do
echo "--- $f"
rg -n -C 8 '(CountDownLatch|CyclicBarrier|await\(|join\(|Executor|replacement|sameUuid|sameUuid|deleteByTenantAndId|findOrStore|blank|contentHash|retryDeadLettered)' "$f" || true
done
printf '\n== public Javadoc on modified API symbols ==\n'
git diff --unified=8 e059824675038fc30e52dc2f356c81b63b6cf3bd^..HEAD -- \
src/main/java \
src/test/java \
docs/security/2026-08-05-administrative-authorization.md \
CHANGELOG.md
printf '\n== audit identifier flow ==\n'
rg -n -C 4 --type java \
'(AdministrativeAudit|actorFingerprint|fingerprint|tenantId\(\)|subject\(\)|jobId|contentHash|filename|message)' \
src/main/java/com/clearfolio/viewer/audit \
src/main/java/com/clearfolio/viewer/security \
src/main/java/com/clearfolio/viewer/controller
printf '\n== whitespace errors ==\n'
git diff --check 5261356ac34e6545bce947ba0bcf2b1ce9f9be67..HEADLength of output: 188142 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== exact concurrency test =='
cat -n src/test/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepositoryConcurrencyTest.java
printf '%s\n' '== complete repository critical-section and index helpers =='
sed -n '1,360p' src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java
printf '%s\n' '== complete administrative audit logger and tests =='
cat -n src/main/java/com/clearfolio/viewer/audit/AdministrativeAuditLogger.java
cat -n src/test/java/com/clearfolio/viewer/audit/AdministrativeAuditLoggerTest.java
cat -n src/test/java/com/clearfolio/viewer/security/AuditPseudonymizerTest.java
printf '%s\n' '== audit contract wording =='
rg -n -C 8 -i \
'(audit evidence|audit output|raw|identifier|jobId|fingerprint|pseudonym)' \
docs/security/2026-08-05-administrative-authorization.md \
AGENTS.md CHANGELOG.md
printf '%s\n' '== retry and lifecycle synchronization paths =='
sed -n '180,325p' src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java
sed -n '1,240p' src/main/java/com/clearfolio/viewer/repository/RepositoryBackedConversionJobStateStore.java
cat -n src/test/java/com/clearfolio/viewer/repository/RepositoryBackedConversionJobStateStoreTest.java
printf '%s\n' '== exact parent-relative diff for concurrency test and repository =='
git diff --unified=20 e059824675038fc30e52dc2f356c81b63b6cf3bd^..310b2ee67dff58ef44b4c10c8b7b07ce878e3dc0 -- \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/test/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepositoryConcurrencyTest.java \
docs/security/2026-08-05-administrative-authorization.md \
CHANGELOG.md
printf '%s\n' '== direct structured audit logging calls outside the administrative logger =='
rg -n -C 3 --type java \
'(LOGGER\.(info|warn|error|debug|trace)\(|logger\.(info|warn|error|debug|trace)\()' \
src/main/java/com/clearfolio/viewer/audit \
src/main/java/com/clearfolio/viewer/controller/AdminController.java \
src/main/java/com/clearfolio/viewer/auth/TenantAccessService.javaLength of output: 103781
MEDIUM — The replacement-race regression test is not deterministic.
In the pre-fix implementation, the replacement can start only after Line 74. Delete cleanup then completes before the replacement index is written. The final lookup passes. The test therefore does not always reproduce the old primary-map/secondary-index interleaving. Make the test require the replacement operation to reach its index-write point before delete cleanup continues. The fixed implementation blocks that operation on I verified these static properties:
I could not run Maven, repository scripts, JaCoCo, or GitHub Actions in this environment. The claimed 100% coverage and runtime test results are unverified. Keep this PR draft. After
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='d4eb774aed8cf897cdf7c8d4fac5211c82d71d9d'
echo '== Head =='
git rev-parse HEAD
git show -s --format='%H%n%P%n%s' HEAD
if [ "$(git rev-parse HEAD)" != "$expected" ]; then
echo "WARNING: checked-out head differs from requested $expected"
fi
echo '== Candidate files =='
fd -t f -a -E target . | rg \
'(AdminController|DocumentConversionService|DefaultDocumentConversionService|ConversionJobRepository|ConversionJobStateStore|InMemoryConversionJobRepository|TenantScoped|Atomic|Stale|Collision|administrative-authorization|CHANGELOG)' | sort
echo '== Production outlines =='
for f in \
src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobStateStore.java \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/controller/AdminController.java; do
if [ -f "$f" ]; then
echo "--- $f"
ast-grep outline "$f" --items all --view compact || true
fi
done
echo '== Scoped mutations and side effects =='
rg -n --type java -C 10 \
'(deleteByTenantAndId\s*\(|retryDeadLetteredForTenant\s*\(|findByTenantAndId\s*\(|deleteArtifact\s*\(|enqueue\s*\(|deleteById\s*\(|retryDeadLettered\s*\()' \
src/main/java
echo '== In-memory shared synchronization and index handling =='
rg -n -C 12 \
'(synchronized|lock|contentHash|tenantContent|findOrStore|deleteByTenantAndId|deleteById|save\s*\()' \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java
echo '== Focused tests =='
fd -t f -a -E target . | rg \
'(TenantScopedAtomic|Stale|Collision|Concurrent|InMemoryConversionJobRepository.*Test|AdminController.*Test).*\.java$' \
| sort | while IFS= read -r f; do
echo "--- $f"
rg -n -C 4 \
'(deleteByTenantAndId|retryDeadLetteredForTenant|same.UUID|replacement|stale|artifact|enqueue|latch|await|assert)' "$f" || true
done
echo '== Documentation atomicity wording =='
rg -n -C 5 \
'(atomic|artifact|enqueue|worker|lifecycle|tenant-scoped|deleteByTenantAndId|retryDeadLetteredForTenant)' \
docs/security CHANGELOG.md AGENTS.md 2>/dev/null || true
echo '== Diff integrity and commits =='
git diff --check HEAD^ HEAD
git show --stat --oneline HEAD
git log --oneline -12Length of output: 192532
The sandbox checked out The inspected descendant closes the prior HIGH finding:
The documentation also limits the atomicity claim to tenant-scoped persistence mutation. It states that artifact cleanup occurs after deletion. It states that worker enqueue occurs after the atomic retry transition. This matches the lifecycle limitation described for I found no evidence in the inspected descendant that the administrative contract claims atomic artifact cleanup or atomic worker dispatch. The remaining UUID-reuse lifecycle risk remains out of scope for this PR and belongs in I did not run Maven or repository script tests. The environment cannot execute repository code. Exact-head Actions evidence remains absent. Keep ✏️ Learnings added
You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='310b2ee67dff58ef44b4c10c8b7b07ce878e3dc0'
actual="$(git rev-parse HEAD)"
printf '== exact head ==\nexpected=%s\nactual=%s\n' "$expected" "$actual"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' HEAD
printf '\n== focused file map ==\n'
fd -t f -a -E target . | rg \
'(InMemoryConversionJobRepository|ConversionJobRepository|ConversionJobStateStore|DefaultDocumentConversionService|DocumentConversionService|InMemoryConversionJobRepositoryConcurrencyTest|TenantScopedAtomic|administrative-authorization|CHANGELOG)' \
| sort
printf '\n== production outlines ==\n'
for f in \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobStateStore.java \
src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java; do
test -f "$f" || continue
echo "--- $f"
ast-grep outline "$f" --items all --view compact
done
printf '\n== shared critical section and scoped mutations ==\n'
rg -n -C 12 --type java \
'(jobIndexLock|synchronized|findOrStoreByContentHash|findByTenantAndContentHash|deleteByTenantAndId|deleteById|retryDeadLetteredForTenant|retryDeadLettered|save\s*\()' \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java
printf '\n== concurrency test ==\n'
cat -n src/test/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepositoryConcurrencyTest.java
printf '\n== atomic mutation tests ==\n'
for f in \
src/test/java/com/clearfolio/viewer/service/TenantScopedAtomicMutationBoundaryTest.java \
src/test/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepositoryTenantScopeTest.java; do
test -f "$f" || continue
echo "--- $f"
rg -n -C 5 '(replacement|CountDownLatch|await|deleteByTenantAndId|retryDeadLetteredForTenant|enqueue|artifact|assert)' "$f"
done
printf '\n== atomicity documentation ==\n'
rg -n -C 6 -i \
'(atomic|artifact|enqueue|worker|UUID|lifecycle|tenant-scoped|deleteByTenantAndId|retryDeadLetteredForTenant)' \
docs/security/2026-08-05-administrative-authorization.md CHANGELOG.md AGENTS.md
printf '\n== post-fix diff and whitespace ==\n'
git diff --unified=12 e059824675038fc30e52dc2f356c81b63b6cf3bd^..HEAD -- \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/test/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepositoryConcurrencyTest.java \
docs/security/2026-08-05-administrative-authorization.md CHANGELOG.md
git diff --check 5261356ac34e6545bce947ba0bcf2b1ce9f9be67..HEADLength of output: 5101 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== production outlines =='
for f in \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobRepository.java \
src/main/java/com/clearfolio/viewer/repository/ConversionJobStateStore.java \
src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java; do
echo "--- $f"
ast-grep outline "$f" --items all --view signatures
done
printf '%s\n' '== repository: lock, index, delete, retry paths =='
rg -n -C 10 --type java \
'(jobIndexLock|synchronized|findOrStoreByContentHash|findByTenantAndContentHash|deleteByTenantAndId|deleteById|retryDeadLetteredForTenant|retryDeadLettered|save\s*\()' \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java
sed -n '1,380p' src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java
printf '%s\n' '== service: scoped mutation and side-effect ordering =='
rg -n -C 12 --type java \
'(deleteJob\s*\([^)]*TenantContext|retryDeadLettered\s*\([^)]*TenantContext|deleteByTenantAndId|retryDeadLetteredForTenant|deleteArtifact|enqueue)' \
src/main/java/com/clearfolio/viewer/service/DefaultDocumentConversionService.java
printf '%s\n' '== deterministic concurrency test =='
cat -n src/test/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepositoryConcurrencyTest.java
printf '%s\n' '== atomic mutation boundary tests =='
for f in \
src/test/java/com/clearfolio/viewer/service/TenantScopedAtomicMutationBoundaryTest.java \
src/test/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepositoryTenantScopeTest.java; do
echo "--- $f"
rg -n -C 6 '(replacement|CountDownLatch|await|deleteByTenantAndId|retryDeadLetteredForTenant|enqueue|artifact|assert)' "$f"
done
printf '%s\n' '== atomicity documentation =='
rg -n -C 6 -i \
'(atomic|artifact|enqueue|worker|UUID|lifecycle|tenant-scoped|deleteByTenantAndId|retryDeadLetteredForTenant)' \
docs/security/2026-08-05-administrative-authorization.md CHANGELOG.md AGENTS.md
printf '%s\n' '== post-fix diff and whitespace =='
git diff --unified=12 e059824675038fc30e52dc2f356c81b63b6cf3bd^..HEAD -- \
src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java \
src/test/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepositoryConcurrencyTest.java \
docs/security/2026-08-05-administrative-authorization.md CHANGELOG.md
git diff --check 5261356ac34e6545bce947ba0bcf2b1ce9f9be67..HEADLength of output: 92842
The prior HIGH read-then-unscoped-mutate finding is closed.
The shared The authorization ADR and changelog do not claim atomic artifact cleanup or worker dispatch. They state that cleanup and enqueue occur after the scoped persistence mutation. This matches the lifecycle limitation assigned to However, one MEDIUM test-quality finding remains.
At Lines 65-72, the test waits one second for Make the test establish a mandatory coordination point for the replacement operation before deletion continues. A controlled repository hook at the lock boundary is suitable. Keep bounded timeouts to detect deadlock. Do not use an optional timeout as proof that the replacement raced with cleanup. I found no static regression in the tenant-scoped mutation boundary, index synchronization, or the stated atomicity scope. I could not run Maven, repository scripts, JaCoCo, or GitHub Actions in this environment. Absent exact-head CI remains unverified. Keep You are interacting with an AI system. |
|
@coderabbitai review @opencode-agent @cwl-noema-review Independently review exact stacked head The exact-head fix removes timeout-as-success. It requires a replacement-task start handshake, verifies through Java 21 Verify the test is deterministic and fails the pre-lock implementation rather than passing through scheduler delay; inspect monitor-owner evidence, cleanup in |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='d6103aa9fafb7476216593fa4026500f099336a1'
actual="$(git rev-parse HEAD)"
printf '== exact head ==\nexpected=%s\nactual=%s\n' "$expected" "$actual"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' HEAD
printf '\n== target file map ==\n'
fd -t f -a -E target . | rg \
'(InMemoryConversionJobRepositoryConcurrencyTest|InMemoryConversionJobRepository|administrative-authorization|CHANGELOG)' | sort
test_file='src/test/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepositoryConcurrencyTest.java'
repo_file='src/main/java/com/clearfolio/viewer/repository/InMemoryConversionJobRepository.java'
printf '\n== test outline ==\n'
ast-grep outline "$test_file" --items all --view expanded
printf '\n== complete replacement-race test ==\n'
cat -n "$test_file"
printf '\n== repository synchronization boundary ==\n'
rg -n -C 14 --type java \
'(jobIndexLock|synchronized|save\s*\(|deleteByTenantAndId\s*\(|remove.*index|contentHash|tenantContent)' \
"$repo_file"
printf '\n== test change at exact head ==\n'
git diff --unified=30 HEAD^..HEAD -- "$test_file" "$repo_file" \
docs/security/2026-08-05-administrative-authorization.md CHANGELOG.md
printf '\n== pre-lock and pre-fix test history ==\n'
git show --format=fuller --stat b47464f3c1a2c75821dba121e090f27b089e178d -- "$test_file"
git show b47464f3c1a2c75821dba121e090f27b089e178d:"$test_file" | cat -n
printf '\n== shared-lock introduction ==\n'
git show --unified=25 e059824675038fc30e52dc2f356c81b63b6cf3bd -- "$repo_file" "$test_file"
printf '\n== Java version and test dependencies ==\n'
rg -n -C 3 '(maven.compiler|<java.version>|source>|target>|ThreadMXBean|ThreadInfo)' \
pom.xml .mvn src/test/java -g '!**/target/**' || true
printf '\n== cleanup and interruption APIs in test ==\n'
rg -n -C 8 '(finally|interrupt|InterruptedException|shutdown|awaitTermination|Future|ExecutionException|TimeoutException|ThreadMXBean)' \
"$test_file"
printf '\n== whitespace ==\n'
git diff --check HEAD^..HEADLength of output: 57991
I found no remaining static issue in the replacement-race regression test. The test now establishes the required interleaving before it releases delete cleanup:
This closes the prior timeout-as-success defect. The two-second bound in The test also fails with the pre-lock implementation. In that implementation, replacement The monitor-owner check is correctly connected to the repository implementation. Cleanup is adequate:
No production code changed in this exact-head commit. I did not run Maven, repository scripts, coverage, or GitHub Actions. Exact-head CI remains absent and unverified. Keep ✏️ Learnings added
You are interacting with an AI system. |
|
Supporting deterministic evidence for exact head
The old no-lock path now fails immediately rather than being accepted after scheduler delay. The fixed shared-monitor path reaches |
|
Superseded by #268. The implementation head remains |
|
Closing as superseded by clean stacked replacement #268. The old stack remained attached to superseded parent #165 and therefore inherited that branch's unrelated Netty/SBOM workflow history. #268 targets clean privacy parent #267 and carries the tenant-scoped authorization implementation on the new dependency line. No review, check, or approval from this closed PR is reused as current evidence; #268 remains draft until #267 integrates, the stack is reconciled onto protected |
|
Closing as superseded by clean stacked replacement #268. The implementation head is the same, but #268 is based on clean privacy parent #267 rather than closed, polluted parent #165. #268 must remain Draft until Netty prerequisite #269 and privacy parent #267 integrate in order, then reconcile onto protected |
1 participant
Security objective
Make every administrative conversion-job endpoint fail closed under signed tenant claims, least-privilege permissions, tenant-scoped persistence queries, atomic tenant-scoped mutations, tenant-bound secondary indexes, and privacy-safe audit evidence. This clean replacement supersedes #172 and remains stacked on parent privacy PR #165.
Shipped security boundary
admin:readis required for tenant-scoped listing;admin:writeis required for delete and retry.findAllByTenantId; the controller never materializes the global job list.false/NOT_FOUND, leaving existing stored state unchanged.X-Clearfolio-*headers before adding canonical signed claims.Test-first evidence
The bounded history includes independent RED→GREEN repairs for tenant-scoped listing, fail-closed adapter defaults, atomic delete/retry, stale secondary indexes, UUID collision, delete-versus-replacement races, and deterministic monitor coordination.
The latest review found that tenant-scoped methods threw
NullPointerExceptionwhen an internal or modular caller omitted the job UUID. Test commit6f522f123465a3fff5de6d47963125097f8da03baddedInMemoryConversionJobRepositoryMissingIdentifierTestbefore production changes. A Java 21 exact-source probe confirmed all three prior paths threwNullPointerException. Commits3db237778536ba9f9ece7eb076559d45dfbfe118and60518a88005b629128fed3bbcc77daf1772b9abfmade scoped lookup/delete/retry return the stable concealed outcomes; the same probe then passed and proved the stored owned job remained present. The temporary read-only RED workflow is absent from the final tree. Authoritative security documentation andCHANGELOG.mdwere updated in3e519b2e7a937b833bcd25539eda65d47761bde6and7737503f44d70f5180f7098554030faba6c73b0b.Exact-head status
Exact current head is
7737503f44d70f5180f7098554030faba6c73b0b, based on current parent #165 head8ca895889abfa30e8fd6dbf0f80d6a467011e6fb.This stacked pull request still has no authoritative exact-head GitHub Actions acceptance evidence because the repository's permanent PR workflows target
main. Local Java 21 probes are supporting evidence only. No absent, queued, pending, cancelled, skipped-required, stale-head, local-only, or predecessor-head result is counted as passing.Merge order and gate
Keep this PR draft. Parent #165 must integrate first. Then reconcile this bounded change onto the resulting protected
mainand require fresh exact-head Maven verification with zero missed production lines and branches, warning-free public Javadocs, CI, Security Scan, SAST, every fuzz job, CodeRabbit, Strix/OpenCode/Noema review, zero unresolved threads, a counted independent write-authorized approval, and every repository protection. Do not temporarily retarget to manufacture checks, use a legacy mutation fallback, weaken tests, or bypass branch protection.