Skip to content

fix(security): require signed tenant claims on admin endpoints - #268

Draft
seonghobae wants to merge 53 commits into
fix/pii-logging-16240128950440010639from
fix/admin-endpoint-auth-clean
Draft

fix(security): require signed tenant claims on admin endpoints#268
seonghobae wants to merge 53 commits into
fix/pii-logging-16240128950440010639from
fix/admin-endpoint-auth-clean

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

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 draft supersedes closed #266 and is stacked on authoritative privacy-and-Netty parent #270 through base branch fix/pii-logging-16240128950440010639. Administrative audit evidence reuses the parent's dedicated keyed pseudonymization contract.

Security boundary

  • require admin:read for tenant-scoped listing and admin:write for delete and retry;
  • return 503 before repository access when the signed-claim HMAC verifier is absent or shorter than 32 UTF-8 bytes;
  • verify missing, malformed, expired, invalid-signature, and insufficient-permission claims through the real authorization service;
  • select administrative list data through findAllByTenantId before job objects cross the repository/service boundary;
  • pass verified TenantContext into delete and retry service mutations;
  • use atomic deleteByTenantAndId and retryDeadLetteredForTenant contracts whose adapter defaults fail closed without global lookup or legacy mutation;
  • conceal missing, null-ID, and cross-tenant targets identically without artifact deletion, state transition, deduplication, or worker enqueue;
  • clean artifacts only after an owned delete succeeds and enqueue work only after an owned retry transition succeeds;
  • bind content-hash indexes to tenant plus hash, validate indexed ownership, remove replaced indexes, reject UUID collisions, and serialize primary/index operations under one shared critical section;
  • emit domain-separated administrative actor and tenant HMAC fingerprints without raw claim, subject, tenant, filename, document, or signature data;
  • remove direct secret-bearing environment binding from the buyer-demo profile and require upstream stripping of untrusted X-Clearfolio-* headers.

Test-first evidence

The branch contains deterministic regressions for tenant-scoped query boundaries, fail-closed compatibility adapters, atomic mutation ordering, every retry outcome, UUID collision and stale-index behavior, same-UUID replacement races, monitor contention, cross-tenant concealment, and privacy-safe audit logging.

The latest bounded repair added InMemoryConversionJobRepositoryMissingIdentifierTest before production changes. A Java 21 exact-source probe proved scoped lookup, delete, and retry previously threw NullPointerException for a missing job UUID. The production contracts now return empty, false, or NOT_FOUND and leave an existing owned job untouched. The temporary RED workflow is absent, and authoritative security documentation plus CHANGELOG.md record the behavior.

Exact-head and stack status

Exact current head is 808e57d6006f1ac332e98ec32d7c156ded4f48dd. Authoritative parent #270 is now exact head 26563218ae42eaa876c784fcf56b27f8cb810080.

A fresh GitHub comparison reports this branch as diverged, 53 commits ahead and 84 commits behind the parent, with merge base 5261356ac34e6545bce947ba0bcf2b1ce9f9be67. The current tree therefore is not merge evidence. No pull-request-triggered GitHub Actions acceptance run exists for this exact head; the successful CodeRabbit commit status is not a substitute for CI, security, formal review, or independent approval.

Required reconciliation

Rebuild the bounded 31-file administrative-authorization slice as a clean descendant of parent exact head 26563218ae42eaa876c784fcf56b27f8cb810080. Preserve the parent's privacy, Netty, deterministic SBOM/attribution, exact-head, zero-coverage-miss, warning-free Javadoc, and fail-closed Maven report-evidence contracts. Reconcile shared CHANGELOG.md, AuditPseudonymizer.java, and AuditPseudonymizerTest.java rather than overwriting parent evidence. Do not add temporary workflows, repair scripts, unrelated dependencies, release changes, or predecessor-parent files.

Merge order and gate

Keep this PR draft. Parent #270 must become stable, pass every exact-head gate, receive counted independent approval, and integrate first. Then reconcile this bounded administrative change onto the resulting protected main and require fresh exact-head Maven verify, zero missed production lines and branches, warning-free public Javadocs, CI, Security Scan, SAST, every required fuzz job, CodeRabbit, Strix/OpenCode/Noema review, zero unresolved threads, a counted independent write-authorized approval, and every repository protection. Do not manufacture checks by temporary retargeting, use legacy mutation fallbacks, weaken tests, or bypass branch protection.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 634f5625-3b0a-4583-ba15-84ddffc90b4d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

@seonghobae
seonghobae changed the base branch from fix/pii-logging-clean to fix/pii-logging-16240128950440010639 August 5, 2026 13:09

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Rebuild this stacked PR cleanly on authoritative parent #270 exact head 06452c3f39f2deb38d31d189e46de4b25512baa0. Current implementation head 808e57d6006f1ac332e98ec32d7c156ded4f48dd is a divergent descendant of the retired parent history and is not merge evidence.

Preserve only the bounded administrative-authorization slice represented by the current 31-file diff: signed-claim verification, least-privilege admin:read/admin:write, tenant-scoped list/delete/retry contracts, concealed missing/cross-tenant outcomes including null UUIDs, atomic repository/state-store mutations, secondary-index consistency, privacy-safe domain-separated administrative audit evidence, config-tree secret loading, OpenAPI/deployment contract, realistic concurrency/security tests, and authoritative documentation. Reconcile shared CHANGELOG.md, AuditPseudonymizer.java, and AuditPseudonymizerTest.java against parent #270 rather than overwriting its Netty evidence. Do not add temporary workflows, repair scripts, unrelated dependency changes, or predecessor-parent files.

Use a clean parent-based branch history or replace the current branch only after the complete tree is ready. Run the strongest available local mvn -B --no-transfer-progress verify and relevant contract tests, keep the PR draft, and report the exact new head. After the clean rebuild, require fresh exact-head CI/security/fuzz/review evidence when the stack can target protected main; do not count current divergent-head or predecessor results.

@seonghobae
seonghobae changed the base branch from fix/pii-logging-16240128950440010639 to fix/pii-logging-clean August 5, 2026 13:11
@seonghobae
seonghobae changed the base branch from fix/pii-logging-clean to fix/pii-logging-16240128950440010639 August 5, 2026 13:15

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Review exact stacked head 808e57d6006f1ac332e98ec32d7c156ded4f48dd against parent #270 exact 06452c3f39f2deb38d31d189e46de4b25512baa0 without treating absent Actions as passing.

Manual review found that both ConversionJobRepository.findByTenantAndId and findByTenantAndContentHash retained adapter defaults that called global lookup and filtered only after materializing a job. That contradicted the branch's fail-closed modular-adapter contract. Test-first commits now prove neither global lookup path is invoked; both defaults return empty until a durable adapter implements scoped predicates, and InMemoryConversionJobRepository explicitly implements owned identifier and tenant+hash lookup. Verify the tests, Javadocs, shared-lock usage, null/blank/missing/cross-tenant concealment, and that no legitimate standalone or administrative caller silently relies on the old global fallback. Update the authoritative administrative-authorization documentation and one existing CHANGELOG.md Security entry if wording is incomplete, without duplicating sections.

Keep Draft. Parent #270 must integrate first; then this branch must be reconciled onto protected main and rerun exact-head Maven verify, zero missed line/branch coverage, warning-free Javadocs, CI, Security, SAST, fuzz, CodeRabbit, Strix/OpenCode/Noema, unresolved-thread, independent-approval, and branch-protection gates.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Parent update: rebuild the tenant-scoped administrative authorization slice directly on #270 exact head 91091ddc212dac328ff36696f56c0a15c21407f4, not retired parent 06452c3f39f2deb38d31d189e46de4b25512baa0. Preserve the bounded 31-file security slice only: signed claims, least-privilege admin:read/admin:write, tenant-scoped list/delete/retry, concealed missing/cross-tenant/null-UUID outcomes, atomic state transitions, tenant-bound secondary indexes, domain-separated privacy-safe audit evidence, config-tree secrets, deployment/OpenAPI contracts, and security/concurrency tests. Reconcile shared AGENTS.md, CHANGELOG.md, AuditPseudonymizer, DefaultDocumentConversionService, InMemoryConversionJobRepository, and their tests against the parent rather than overwriting #270's audit-key, Netty/SBOM, filesystem TOCTOU, zero-coverage, or warning-free-Javadoc contracts. Use a clean parent-based history, keep the PR draft, run mvn -B --no-transfer-progress verify, and report the exact rebuilt head. Do not add temporary/write-scoped workflows or count current divergent-head/predecessor evidence.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Rebuild this draft as a clean descendant of authoritative parent #270 exact head 26563218ae42eaa876c784fcf56b27f8cb810080.

Fresh GitHub comparison shows current head 808e57d6006f1ac332e98ec32d7c156ded4f48dd is diverged: 53 commits ahead, 84 behind, merge base 5261356ac34e6545bce947ba0bcf2b1ce9f9be67. Preserve only the bounded 31-file administrative-authorization slice: signed tenant claims; least-privilege admin:read/admin:write; tenant-scoped list/delete/retry; atomic owned mutations; tenant-bound indexes; null/missing/cross-tenant concealment; privacy-safe domain-separated audit evidence; config-tree secret loading; deployment/OpenAPI contracts; deterministic security and concurrency tests; and authoritative documentation.

Reconcile shared CHANGELOG.md, AuditPseudonymizer.java, and AuditPseudonymizerTest.java without overwriting the parent's privacy, Netty 4.1.136.Final, deterministic SBOM/attribution, exact-head, zero-missed-line/branch, warning-free Javadoc, and fail-closed Maven report-evidence contracts. Do not add temporary workflows, repair scripts, unrelated dependencies, release changes, or predecessor-parent files.

Implement test-first, run the strongest available mvn -B --no-transfer-progress verify and relevant contract tests, keep the PR draft, and report the exact rebuilt head. Do not count the current CodeRabbit status or absent Actions as acceptance evidence.

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