fix(cdc): rebuild DDL log confidentiality on live develop - #267
fix(cdc): rebuild DDL log confidentiality on live develop#267seonghobae wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthrough스키마 변경 처리 로그에서 원시 DDL과 예외 진단 정보를 제거했습니다. 성공, 차단, 중복, 실패, JSON 파싱 실패 결과를 제한된 메타데이터로 기록합니다. 새 테스트는 각 상황의 로그와 JDBC 동작을 검증합니다. Changes스키마 변경 로깅
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cdc-service/src/test/java/com/xtrmetl/cdc/replication/SchemaChangeReplicaApplierLoggingTest.java (1)
27-28: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win차단 경로의 기밀성 테스트를 추가하십시오.
requireSingleStatement와requireCommentFree의 로그 경로는 현재 테스트되지 않습니다. 다중 문장, SQL 주석, NUL 입력마다verifyNoInteractions(jdbcTemplate)와 원시 DDL 비노출을 검증하십시오. 각 경우에"Blocked multi-statement DDL"또는"Blocked DDL containing SQL comments or NUL"로그 분류도 검증하십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cdc-service/src/test/java/com/xtrmetl/cdc/replication/SchemaChangeReplicaApplierLoggingTest.java` around lines 27 - 28, Extend SchemaChangeReplicaApplierLoggingTest to cover the requireSingleStatement and requireCommentFree blocked paths for multi-statement, SQL-comment, and NUL inputs; verify each case performs no interactions with jdbcTemplate, does not expose the raw DDL in captured logs, and emits the corresponding “Blocked multi-statement DDL” or “Blocked DDL containing SQL comments or NUL” classification.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@cdc-service/src/test/java/com/xtrmetl/cdc/replication/SchemaChangeReplicaApplierLoggingTest.java`:
- Around line 27-28: Extend SchemaChangeReplicaApplierLoggingTest to cover the
requireSingleStatement and requireCommentFree blocked paths for multi-statement,
SQL-comment, and NUL inputs; verify each case performs no interactions with
jdbcTemplate, does not expose the raw DDL in captured logs, and emits the
corresponding “Blocked multi-statement DDL” or “Blocked DDL containing SQL
comments or NUL” classification.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d2d3230-83b9-4c25-aca6-ab315e1f82bc
📒 Files selected for processing (2)
cdc-service/src/main/java/com/xtrmetl/cdc/replication/SchemaChangeReplicaApplier.javacdc-service/src/test/java/com/xtrmetl/cdc/replication/SchemaChangeReplicaApplierLoggingTest.java
Rebuilds stale Draft #171 from exact protected
develop@942d164ceb7af497bb28a95f842db96429d1bdc4without rewriting predecessor history. Keep Draft.Exact current identity
develop@942d164ceb7af497bb28a95f842db96429d1bdc4(fresh compare immediately before this update: identical);repair/schema-ddl-log-redaction-942d164;9c724e3f6f888827157af470aa100fbe709ceb26;017d1355dc04876a0f03e0d655b7fd3a4d3a9d9b;45e140969977299a5cfe7cac2fe6b0e8689b034f;b0fd25a2511b8a0765f46f14ead184858f720cdf;Any check or review from another head/base is historical and does not transfer.
RCA and valid RED
Protected
SchemaChangeReplicaApplierlogged raw normalized DDL on success, duplicate, policy-block and execution-failure paths, and attached parser/database exceptions to ordinary lifecycle diagnostics. DDL and provider exception text can contain schema names, literals, JDBC/internal topology, driver diagnostics or secret-adjacent values. Stable event classification and topic/policy metadata are sufficient for routine operational correlation; replaying raw DDL/provider diagnostics into logs is unnecessary data propagation.Test-only head
9c724e3f6f888827157af470aa100fbe709ceb26added the logging contract. CI run31461114884reached the real schema-replication boundary and produced five intended failures showing raw DDL/private-path/provider diagnostic exposure. That is valid fail-first evidence at the production boundary, while its synthetic checkout remains development evidence rather than literal-source acceptance proof.Narrow GREEN and review follow-up
017d1355dc04876a0f03e0d655b7fd3a4d3a9d9bremoved raw DDL/provider exception transport from routine schema-apply logs while preserving execution, validation, idempotence and exception-propagation behavior. Logs retain bounded outcome classification, topic, validation mode where applicable, and no raw DDL payload.CodeRabbit submitted one
COMMENTEDreview against the older reviewed span ending at017d1355dc04876a0f03e0d655b7fd3a4d3a9d9b, asking for confidentiality coverage of multi-statement, SQL-comment and NUL block paths. Exact current head45e140969977299a5cfe7cac2fe6b0e8689b034fcontains that follow-up: the logging contract now exercises those block paths, verifies no JDBC interaction, verifies raw DDL non-disclosure, and checks the stable block classifications. The current review-thread query returns zero unresolved inline threads. The old reviewed head is not independent approval of the current head.Current hosted proof
All exposed workflow aggregates associated with exact current head
45e140969977299a5cfe7cac2fe6b0e8689b034fare terminal-success:31466534716;31466534729;31466534706;31466534727;31466534717.CI macOS job
93700486046checked out synthetic mergeb0fd25a2511b8a0765f46f14ead184858f720cdf(Merge 45e140... into 942d164...), not the literal source head. On that integration tree ETL passed 276/276, CDC passed 120/120,SchemaChangeReplicaApplierLoggingTestpassed 8/8, and the six-module reactor completedBUILD SUCCESS. The exercised logs emit only bounded classifications on the success, duplicate, policy-block, malformed-event, execution-failure, multi-statement, SQL-comment and NUL paths.The same CI still reports
Analyzed bundle 'etl-service' with 0 classesimmediately before declaring coverage success. #162/#164 owns selected-bundle non-vacuity and #205 owns repository-wide owned-production coverage. Zero-class coverage is not accepted evidence. Shared Maven/scanner dependency materialization completeness remains owned by #196, and aggregate security success does not override that evidence boundary.Predecessor preservation / merge boundary
Do not close #171 yet. Fresh exact-file comparison found unique predecessor work that current #267 has not preserved: #171 carries additional public constructor/apply Javadocs and a private
@Nullablecontract onextractDdl(with its associated annotation import). Therefore unique-work preservation is not proven, so #171 remains a live predecessor rather than a superseded duplicate.Keep this PR Draft. Merge only on an unchanged exact source head after accepted literal-source deterministic/security evidence, complete same-revision dependency/vulnerability evidence, applicable non-vacuous 100% owned-production statement/branch coverage, every required current gate, zero valid unresolved findings, and qualifying independent non-author formal APPROVED review where governance requires it. Synthetic-only, stale/predecessor, status-only, skipped-required, incomplete-scanner, author-only or absent-review evidence is not merge authority.