Skip to content

fix(privacy): keep ETL resource IDs out of failure logs - #219

Closed
seonghobae wants to merge 3 commits into
developfrom
privacy/etl-problem-log-resource-id-622e5e6
Closed

fix(privacy): keep ETL resource IDs out of failure logs#219
seonghobae wants to merge 3 commits into
developfrom
privacy/etl-problem-log-resource-id-622e5e6

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Purpose

Fix #218 test-first from exact protected develop@622e5e6c3d534f230c390f10e3832efadfc01825. The shared EtlApiProblemHandler historically logged raw request.getRequestURI() values for covered ETL failures. Because the same advice handles owner-scoped durable-job controllers, ordinary WARN/ERROR logs could contain opaque per-job resource identifiers even though the HTTP Problem Details body is deliberately bounded.

This direct-develop Draft changes only the shared handler and one focused logging regression. It does not change RFC 9457 response instance, authorization, controller routing, payload retention, status/error-code semantics, local debug profiles, dependencies, or canonical #149 documentation.

Exact current identity

  • protected/live base: develop@622e5e6c3d534f230c390f10e3832efadfc01825;
  • branch: privacy/etl-problem-log-resource-id-622e5e6;
  • fail-first source head: 1679d0c9ad99297ba710425e700fe68c21101dd4;
  • exact current GREEN source head: 79e8e4e7d593665c99d94dfc35fe842c1e5c749f;
  • current synthetic merge preview: 71c9944bdd8102994cf7ff0552db6b4bb9c2119c;
  • exact RED→GREEN comparison: one commit ahead, zero behind, merge base exactly the RED head;
  • changed paths: exactly EtlApiProblemHandler.java and EtlApiProblemHandlerLoggingTest.java;
  • Draft / mergeable: true / true;
  • formal reviews: none;
  • unresolved review threads: zero.

Every check/review from the RED head, predecessor source, another PR, or another base is historical and does not transfer.

RCA

  • Immediate cause: covered failure logs interpolated request.getRequestURI().
  • Technical root cause: client-facing resource identity and operator failure classification were coupled even though the resource path is unnecessary to distinguish stable ETL error classes.
  • Control failure: existing controller tests proved response confidentiality but did not capture ordinary logs for owner-scoped resource paths.
  • Product impact: durable job IDs became unnecessary high-cardinality diagnostic dimensions that can be exported to centralized logs/support bundles, contradicting [Operability/Privacy] Separate local-debug observability from production-safe defaults #182's production-safe observability/privacy boundary.

RED — hosted production-boundary proof

Test-only 1679d0c9ad99297ba710425e700fe68c21101dd4 added EtlApiProblemHandlerLoggingTest before production changed. It invokes the real target-failure handler with /api/etl/jobs/0198f4cf-41c8-7f52-9e5d-private-job-marker and requires stable ETL target failure classification while the unique owner-scoped resource marker remains absent from captured ordinary logs.

CI 31366659260, macOS job 93386394718, checked out synthetic merge 30ccf21c5fcde284385f9d348b35c3020f9a480a (Merge 1679d0c... into 622e5e6...). Production/test compilation succeeded and ETL ran 276 tests with exactly one failure, zero errors/skips. The emitted line contained:

ETL target failure path=/api/etl/jobs/0198f4cf-41c8-7f52-9e5d-private-job-marker ...

and exactly the new no-resource-ID assertion failed. Existing synchronous and durable-job controller failure tests remained green. This is valid RED at the real handler/logging boundary, not setup/import/fixture/dependency failure.

GREEN — exact current source

79e8e4e7d593665c99d94dfc35fe842c1e5c749f applies the narrowest correction:

  • request-URI transport is removed from all five ordinary log calls owned by EtlApiProblemHandler;
  • stable request/error classifications remain;
  • existing exception-type metadata remains where already used;
  • ProblemDetail.instance still uses request.getRequestURI(), so RFC 9457 client semantics are unchanged;
  • no hashes, regex masking, payloads, principals, idempotency keys, SQL, exception messages or new diagnostic identifiers are introduced;
  • class Javadoc documents the ordinary-log resource-identity boundary.

The implementation was published as exactly one fast-forward commit over the observed RED head. The immediate compare is ahead 1 / behind 0 with only EtlApiProblemHandler.java changed, so no concurrent branch movement was silently incorporated.

Exact-current-head hosted verification

All exposed workflow aggregates associated with exact current source 79e8e4e7d593665c99d94dfc35fe842c1e5c749f are terminal-success:

  • CI 31366864646: success on Ubuntu, macOS and Windows;
  • Dependency Review 31366864649: success;
  • CycloneDX SBOM 31366864621: success;
  • SAST Semgrep 31366864623: success;
  • Security Scan 31366864701: aggregate success.

This remains development/merge-preview evidence rather than protected merge authorization. Current CI checked GitHub's synthetic merge preview rather than accepted literal source. Hard Trivy job 93387023450 likewise checked synthetic merge 71c9944... and explicitly warned Dependency version cannot be determined. Child dependencies will not be found. before reporting zero CRITICAL/HIGH/MEDIUM findings; #196 owns that dependency-graph completeness defect. The scan also still encounters the tracked 82 MB zipkin.jar, separately owned by #168/#169.

Protected JaCoCo source remains an independent false-green boundary until #162/#164 or equivalent integration, with repository-wide production coverage scope tracked by #205. A green aggregate cannot change the revision, dependency graph, or selected production units actually examined.

Documentation / relationship

#182 owns the broader production-safe observability profile and finite-cardinality policy; #159 owns canonical Security/Threat Model/Test Strategy/Operability/Traceability reconciliation. This branch closes only the concrete shared-handler log boundary and does not race #149's canonical documentation writer.

Merge boundary

Keep Draft. Merge only when the unchanged exact source head has accepted literal-source deterministic/security evidence, complete same-revision dependency/vulnerability evidence with no unresolved #196 incompleteness, non-vacuous applicable owned-production coverage under #162/#164/#205 or equivalent accepted controls, every required repository/security gate, zero valid unresolved findings, and qualifying independent non-author formal approval where governance requires it. No predecessor-head, other-PR, incomplete-scanner, status-only, skipped-required, queued, or synthetic-merge-only evidence transfers.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

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: 9639c91d-f385-4ae4-8625-6bdb54b882ac

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

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.

Copy link
Copy Markdown
Collaborator Author

Superseded by current-base replacement #320. Fresh pre-disposition verification found #219 at 92b29a0c0dbebdab1ff8d8d7d2490fb3acfc25ef, #320 at 64d186351704ab6993343a5a8dc9b16f73501029, and protected develop@8f96517987cf2cab4398696503b22a131a4b66a6. Both PRs change exactly the same two paths, and the current blobs are identical in both lines: EtlApiProblemHandler.java = 81a0e4892ef74dbc399de165b1d30db795532d83; EtlApiProblemHandlerLoggingTest.java = 2074d59012defb5fed10d88742238f17c8b20be7. No unique work remains only on #219. Checks, reviews, approvals, and old-base evidence do not transfer; #320 remains the Draft replacement requiring its own current-head/current-base acceptance.

@seonghobae seonghobae closed this Aug 13, 2026
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.

[Privacy/Observability] Keep owner-scoped resource identifiers out of ETL failure logs

1 participant