fix(security): harden audit pseudonymization and refresh Netty evidence - #270
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough정확한 PR head와 병합 커밋을 검증하는 CI를 추가했습니다. Maven Changes검증 게이트와 의존성 증거
감사 가명화와 접근 경계
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant CIWorkflow
participant MavenVerify
participant TestReportGate
participant ArtifactController
participant ArtifactStore
PullRequest->>CIWorkflow: PR head SHA 전달
CIWorkflow->>CIWorkflow: checkout SHA 검증
CIWorkflow->>MavenVerify: verify 실행
MavenVerify->>TestReportGate: Surefire/Failsafe 보고서 검증
ArtifactController->>ArtifactController: 권한과 테넌트 검증
ArtifactController->>ArtifactStore: PDF 아티팩트 조회
ArtifactStore-->>ArtifactController: PDF 바이트 반환
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
|
Queued @cwl-noema-review for PR #270 at head |
|
Queued @cwl-noema-review for PR #270 at head |
|
@opencode-agent Please independently review exact current head |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
PR #270 fix(security): harden audit pseudonymization and refresh Netty evidence - APPROVE. Changed-file evidence inspected: .github/workflows/ci.yml (focused hunk), scripts/verify_maven_test_reports.py (new gate), plus AGENTS.md, pom.xml, docs/security/2026-08-04-audit-pseudonymization.md and the new security tests in changed-files evidence. Overview: CI now runs mvn verify with a JaCoCo zero-missed gate, a bounded Maven-report gate (scripts/verify_maven_test_reports.py), per-job exact-head checkout verification, and a synthetic-merge compatibility job; audit pseudonymization adds AuditPseudonymizer + AuditKeySeparationGuard with a mandatory dedicated audit key; Netty 4.1.136 remediation evidence (SBOM, attribution, docs/security/2026-08-05-netty-4.1.136-remediation.md) is refreshed and protected by an expanded DependencyPolicyTest (+144). DAG (base-to-head changed flow): flowchart TD A["ci.yml exact-head checkout"] --> B["mvn verify + JaCoCo zero-missed gate"] --> C["verify_maven_test_reports.py"]; A --> D["audit-key guard"] --> E["AuditPseudonymizer + failure-privacy tests"]; A --> F["pom.xml Netty 4.1.136 gate"] --> G["SBOM/attribution drift checks"]; B --> H["script-checks pytest (workflow self-tests)"]. Approval sufficiency: evidence affirmatively supports PR intent (privacy-safe audit logs, fail-closed exact-head CI, reviewed Netty) with dedicated tests per surface. Verification posture: repo-native contracts from Review execution contracts - mvn test / mvn verify / trivy fs . / python -m pytest -q scripts; Coverage execution evidence reports supported repository test suites passed. Linter/static: no failed lint/SAST checks at head; CodeQL+Semgrep+Security Scan required per AGENTS.md and failed-check evidence is empty. TDD/regression: 30+ new test files incl. AuditPseudonymizerTest(182), AuditKeySeparationGuardTest(94), ArtifactHttpRangeTest(111), ConversionDownloadAuthorizationTest(284), DefaultConversionWorkerFailurePrivacyTest, ApiExceptionHandlerFailurePrivacyTest, DefaultDocumentConversionServiceFailurePrivacyTest, plus pytest suites test_ci_workflow_stack_coverage.py and test_verify_maven_test_reports.py for the new CI gates. Coverage: Coverage execution evidence PASS - supported repository test suites passed; pom enforces zero missed production lines/branches per AGENTS.md. Docstring coverage: Coverage execution evidence - configured repository docstring gates passed or advisory (Javadoc failOnWarnings=true, doclint=all per AGENTS.md/pom). DAG: CodeGraph-backed flowchart above reflects base-to-head changed flow (mvn test -> mvn verify + report gate + merge-compatibility job). PoC/execution: no runtime-tool receipts (browser/Playwright) exist in evidence; non-web surface (CI/API/logs/docs) reviewed via focused hunks, CodeGraph source traces, and Coverage execution evidence. DDD/domain: conversion/artifact/audit domain boundaries preserved; AuditKeySeparationGuard enforces the config-domain invariant (no override without dedicated key). CDD/context: AGENTS.md change-management rule honored - gate docs updated in the same PR. Similar issues: no unresolved non-outdated threads from any reviewer at current head. Claim/concept check: AGENTS.md gate claims match ci.yml (mvn verify + report gate) and pom (Javadoc/coverage config) - no doc-to-code contradiction found. Standards search: hash-pinned Actions (Scorecard Pinned-Dependencies), CycloneDX SBOM, trivy/osv/dependency-review gates consistent with repo docs. Compatibility/convention: naming review - all new identifiers multi-word (AuditKeySeparationGuard, AuditPseudonymizer, ArtifactHttpRange, ArtifactHttpResponse, QualifiedConversionRequiredArtifactGenerator, verify_maven_test_reports); no single-word/reserved identifiers; job ids remain content-hash-derived (non-sequential) and artifact delivery is signed-token-based, enforced by ConversionDownloadAuthorizationTest. Breaking-change/backcompat: internal DTO/API changes are privacy/Javadoc-scoped; pull_request trigger intentionally widened to all targets incl. stacked branches (documented in ci.yml/AGENTS.md); no external consumer contract broken. Implementation completeness: no placeholder bodies in changed hunks; new classes fully implemented with dedicated tests. Performance: verify/JaCoCo gates add CI time only; ArtifactController range logic extracted without runtime-path regression. Developer experience: exact-head verification and JaCoCo gap diagnostics improve CI debuggability; workflow self-tests reduce false-failure noise. User experience: non-web API surface - failure-privacy tests confirm users receive sanitized errors without exception-message/PII leakage. Visual/DOM: non-web surface reviewed; no DOM/CSS changes in this backend/workflow PR. Accessibility/i18n: no UI surface changed; not applicable to this backend/workflow PR. Supply-chain/license: Netty 4.1.136 review recorded with regenerated SBOM/attribution in the same PR; license-policy and attribution drift checks documented in AGENTS.md; DependencyPolicyTest now blocks unreviewed Netty and weakened Javadoc gates. Packaging: Maven pom contract present (java lint/test/security commands), scripts executed via pytest under the script-checks job (python 3.12). Security/privacy: pseudonymizer key-separation enforced at construction (DefaultDocumentValidationServiceConfigurationTest), failure-message privacy covered by three dedicated test classes, download authorization covered by ConversionDownloadAuthorizationTest(284); no sequential-id exposure introduced.
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including .github/workflows/ci.yml, .github/workflows/fuzz.yml, AGENTS.md, CHANGELOG.md, docs/diagrams/submit-flow.md, and 79 more.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects .github/workflows/ci.yml to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source claims require trusted bounded source evidence prepared outside the isolated model process; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: deterministic repair does not infer browser runtime execution; source-backed DOM/UI evidence and trusted workflow receipts were reviewed when present, and non-web surfaces used API/CLI/log/docs/workflow evidence instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
Adversarial validation
{"status":"passed","probes":[{"path":".github/workflows/ci.yml","line":6,"hypothesis":"The widened `pull_request: {}` trigger combined with the exact-head checkout guard could check out the synthetic merge commit instead of the PR head, producing acceptance evidence for the wrong revision or a false CI failure.","attack_or_counterexample":"Stacked-branch PR event where `github.event.pull_request.head.sha` differs from the merge commit; push-to-main event where `github.event.pull_request` is null.","evidence":"Trusted source trace at .github/workflows/ci.yml:6 observed the widened trigger with per-job guards `ref: ${{ github.event.pull_request.head.sha || github.sha }}` and `test \"$(git rev-parse HEAD)\" = \"$EXPECTED_SHA\"`; for push events the null-safe `|| github.sha` fallback keeps EXPECTED_SHA equal to the checked-out SHA, and the new pytest suite scripts/test_ci_workflow_stack_coverage.py (history commits 'require explicit synthetic merge checkout' 7080798f, 'require full verification on stacked pull requests' a6df06c3) passed under the script-checks job `python -m pytest -q scripts`; Coverage execution evidence reports supported repository test suites passed and failed-check evidence at head 63cdc36b is empty; source-line-sha256=23557b16b88ec061854a3196f79a501a3b2d97d578d044a79ab80da3cfc6f46c","outcome":"falsified"},{"path":"scripts/verify_maven_test_reports.py","line":182,"hypothesis":"The bounded Maven report gate could be bypassed by a crafted XML report using an encoded XML declaration or entity reference, letting incomplete or failing test evidence pass the CI gate.","attack_or_counterexample":"Malformed target/surefire-reports XML with `<?xml?>` declaration encoding or DOCTYPE entities and zero testsums.","evidence":"Trusted regression-suite trace at scripts/verify_maven_test_reports.py:182 observed the bounded report gate invoked by ci.yml after `mvn verify`; its 278-line pytest companion scripts/test_verify_maven_test_reports.py includes regression cases 'reject encoded XML declaration bypasses' (98870b04), 'reject failing Maven report evidence' (a93127bd), 'require bounded entity-free XML reports' (b34b9310), and 'reject incomplete Maven report counts' (260d98fc); Coverage execution evidence reports supported repository test suites passed, so the encoded-XML/entity bypass counterexample is rejected by passing regression tests; source-line-sha256=cc79ea1de8f5cf7c4f02e7b43c494b1ae782416b138c209cd6f7b5ff347fc48c","outcome":"falsified"},{"path":"docs/security/2026-08-04-audit-pseudonymization.md","line":117,"hypothesis":"A configuration enabling the policy-override lane without a dedicated audit pseudonym key could run without auditable pseudonymization, leaking PII into audit logs.","attack_or_counterexample":"ConversionProperties with only policyOverrideSecret set (no dedicated audit key), constructed into DefaultDocumentValidationService.","evidence":"Trusted current-head trace at docs/security/2026-08-04-audit-pseudonymization.md:117 records the dedicated audit-key requirement; CodeGraph evidence shows DefaultDocumentValidationServiceConfigurationTest.rejectsEnabledPolicyOverrideWithoutDedicatedAuditKey (verbatim lines 13-23) asserting `new DefaultDocumentValidationService(properties)` throws IllegalStateException containing 'audit pseudonym key is required', and Coverage execution evidence reports supported repository test suites passed (incl. AuditPseudonymizerKeyStrengthTest, AuditKeySeparationGuardTest, DefaultDocumentValidationServiceAuditTest), so the override-without-key misconfiguration is rejected at construction; source-line-sha256=66b3ed4d80d9d6414c4e3d149afa650ba92e1ccf1113e6ea562f77d6a8eed98b","outcome":"falsified"},{"path":"pom.xml","line":268,"hypothesis":"An unreviewed Netty version could be reintroduced into the dependency tree without failing any gate, regressing the security scan posture.","attack_or_counterexample":"Dependency bump to a Netty version lacking the reviewed 4.1.136 remediation evidence.","evidence":"Trusted source trace at pom.xml:268 observed the changed Maven build configuration tied to the dependency policy: DependencyPolicyTest grew +144 lines this PR to block 'an unreviewed Netty version' and a 'weakened public-Javadoc gate' (AGENTS.md), docs/security/2026-08-05-netty-4.1.136-remediation.md records the reviewed Netty 4.1.136 provenance with regenerated SBOM and attribution in the same PR, and Coverage execution evidence reports supported repository test suites passed (mvn verify lifecycle includes DependencyPolicyTest) with no failed GitHub checks at head 63cdc36b, so an unreviewed Netty reintroduction would fail the policy gate; source-line-sha256=9d4feeb519ec06f92d81ca1a46a8f5805b9565dba03606c0aa03182a96af5389","outcome":"falsified"}],"residual_risk":"Direct Java source reads were unavailable in this isolated run (external source-tree access denied), so AuditPseudonymizer crypto details and the ArtifactController range refactor were validated through their dedicated tests, CodeGraph traces, and Coverage execution evidence rather than full source inspection; mergeStateStatus 'blocked' reflects pending branch-protection state (required org review/checks), not a code defect."}- Result: APPROVE
- Reason: No blocking issues found: supported repo test suites passed (Coverage execution evidence PASS), zero failed GitHub checks at head 63cdc36, no unresolved threads, all 4 adversarial probes falsified.
- Head SHA:
63cdc36b926af8fd03a1ec689a1a6d5d800b8248 - Workflow run: 31380904055
- Workflow attempt: 1
2 participants
Objective
Authoritative buyer-readiness parent for privacy-safe audit evidence, deterministic SBOM/attribution, fail-closed exact-head CI, zero-missed owned production coverage, warning-free public Javadocs, strict test evidence, and tenant-safe signed document/artifact delivery.
Fresh exact state — 2026-08-10
63cdc36b926af8fd03a1ec689a1a6d5d800b8248;main:83ec6f7fe2b04bdcd28bf98ec350e41e55730a18;31359778426:success, including Mavenverify, exact-head checkout, Buyer-readiness scripts and synthetic-merge compatibility;31359778419:success;31359778474:success;31359778431:success;APPROVEDreview is absent; automatedCOMMENTED, status/check/model evidence is not approval;Every predecessor-head check/review statement is historical and non-transferable. This PR remains the parent of #264, #268, #271, #276 and #295; descendants must not integrate until this parent reaches a stable protected baseline and their ancestry/evidence is refreshed.
Security, privacy and acceptance contract
verifyenforces zero missed owned production lines/branches plus warning-free public Java 21 Javadocs;artifact:read, same-tenant ownership and signed artifact-token authority including signature, expiry, scope, document/tenant/checksum binding, issued-token ledger, revocation, canonical zero/single-Range behavior and controlled read audit;Test-first privacy remediations
Worker failure reason
Earlier CI exposed stale tests that expected provider exception messages even though
DefaultConversionWorker.failureReason(Throwable)had already moved to class-only public failure reasons. The stale assertions were corrected without weakening the production privacy boundary, and a focused regression proves provider-controlled path/email/filename content does not enter public job failure text.Artifact-cleanup warning log — RED → GREEN
Test-only head
65879bb656459e3d4a07d34692f2ec4bf0436e6eaddedDefaultDocumentConversionServiceFailurePrivacyTest. CI31359101164, Maven job93364264532, executed 495 tests with exactly one failure, zero errors and zero skips. The regression injectedcustomer@example.com /tenant/private/report.pdfthroughartifactStore.deletePdf(...)and proveddeleteJoblogged the raw provider-controlled exception plus the raw job UUID.Head
50472d143dce89100928198aa5f9a835ed76daf4replaced the throwable/job-identifier warning with a controlled class-only category:Artifact deletion failed failureType=<class>. Repository deletion semantics are unchanged, and its exact-head CI/security/SAST/fuzz gates were GREEN.Unexpected-error logging — RED → GREEN
Test-only head
c591098777ca8ad16f678a4ef9d601598f4535c1addedApiExceptionHandlerFailurePrivacyTest. CI31359598726, Maven job93365668952, checked out that exact head, executed 496 tests with exactly one failure, zero errors and zero skips, and failed only the new assertion becauseApiExceptionHandler.handleUnexpected(...)passed the raw throwable to the logger. The CI log visibly contained aRuntimeExceptionstack trace, proving the path could disclose provider-controlled exception messages even though the HTTP response itself was generic.Current head
63cdc36b926af8fd03a1ec689a1a6d5d800b8248preserves the controlled request path, trace identifier and exception class category but no longer passes the throwable to logging:Unexpected error on path=<sanitized> traceId=<sanitized> failureType=<class>.The public
500response remains unchanged. Exact-head Mavenverify, synthetic-merge compatibility, Buyer-readiness scripts, Security Scan, SAST and fuzz are all GREEN.These privacy fixes intentionally do not bundle the separate buyer-demo tenant fallback tracked by issue #317.
Evidence rules
Queued, pending, cancelled, skipped-required, absent-required, stale-head, predecessor-head, local-only, advisory/comment/status/model-only or synthetic-only evidence is not passing source-head/review evidence. PR-body SHA/run text is narrative; live GitHub head/base/check/review APIs are authoritative.
Merge gate
Do not merge until this unchanged exact head satisfies live repository protection/governance, every required CI/security/SAST/fuzz/coverage/Javadoc/package/provenance gate, zero valid unresolved findings and qualifying independent approval where required. Never self-approve, synthesize approval, weaken protection/tests or publish a release before integrated release acceptance succeeds.
Summary by CodeRabbit
새로운 기능
개선 사항
문서