Skip to content

Fix: Consumer PII sent to MySQL over an unencrypted connection and written to DEBUG SQL logs - #312

Open
WesternConcrete wants to merge 2 commits into
masterfrom
devin/1788073992-encrypt-mysql-transit-and-trim-sql-logs
Open

WesternConcrete wants to merge 2 commits into
masterfrom
devin/1788073992-encrypt-mysql-transit-and-trim-sql-logs

Conversation

@WesternConcrete

@WesternConcrete WesternConcrete commented Aug 30, 2026

Copy link
Copy Markdown

Summary

Finding: Consumer PII sent to MySQL over an unencrypted connection and written to DEBUG SQL logs (COMPLIANCE / NS terms — data-flow and boundary gap) in COG-GTM/ftgo-monolith.

Consumer names (PersonName) and delivery addresses (Address) crossed the app→MySQL link in cleartext, and Hibernate mirrored the same statements into application stdout, which the container log pipeline collects. Fix requires TLS on every app-owned datasource URL and drops SQL statement logging:

- .../ftgo?useSSL=false&allowPublicKeyRetrieval=true   # application.properties
- .../ftgo?useSSL=false                                # ftgo-flyway/build.gradle
- SPRING_DATASOURCE_URL: jdbc:mysql://mysql/ftgo       # docker-compose.yml
+ ...?sslMode=REQUIRED                                 # all three
- logging.level.org.hibernate.SQL=DEBUG
+ logging.level.org.hibernate.SQL=WARN

allowPublicKeyRetrieval is removed rather than kept: it only exists to let caching_sha2_password auth fall back over an insecure channel, which sslMode=REQUIRED makes unnecessary. Test-scoped properties are left untouched.

Link to Devin session: https://app.devin.ai/sessions/7c734a49fa1849bc9ef496bfc9c72d10
Open in Devin Desktop: https://app.devin.ai/desktop/session/7c734a49fa1849bc9ef496bfc9c72d10?variant=devin
Requested by: @WesternConcrete


Devin Review

…mer data

Co-Authored-By: Wes Convery <2wconvery@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Devin Review

Comment thread docker-compose.yml
logging.level.org.hibernate.SQL=DEBUG
spring.datasource.url=jdbc:mysql://${DOCKER_HOST_IP:localhost}/ftgo?useSSL=false&allowPublicKeyRetrieval=true
logging.level.org.hibernate.SQL=WARN
spring.datasource.url=jdbc:mysql://${DOCKER_HOST_IP:localhost}/ftgo?sslMode=REQUIRED

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟨 Database identity remains unverified

sslMode=REQUIRED encrypts traffic but accepts any server certificate without hostname verification. A network interceptor can impersonate MySQL and capture consumer data and credentials.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct that sslMode=REQUIRED is encryption without server authentication. I kept it at REQUIRED deliberately rather than moving to VERIFY_CA/VERIFY_IDENTITY, because verification needs trust material this repo does not have:

  • The bundled server's certificate is auto-generated per container by the MySQL entrypoint (self-signed, CN MySQL_Server_..._Auto_Generated_Server_Certificate), so there is no stable CA to pin and VERIFY_IDENTITY would fail hostname matching against mysql.
  • Making verification work requires a committed dev CA plus a server cert issued for the mysql hostname and a client truststore (trustCertificateKeyStoreUrl) — and, for real deployments, per-environment CA distribution rather than anything checked into the repo.

That is a separate change with its own surface (cert lifecycle, truststore wiring for both the app and Flyway, k8s/deployment config), so this PR closes the plaintext-transit gap and leaves identity verification as follow-up. Happy to do it here instead if you'd prefer it in scope.

Co-Authored-By: Wes Convery <2wconvery@gmail.com>
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