Skip to content

demo(05-pii-logging): 05 — PII logged in payment success path#154

Open
EricCogen wants to merge 1 commit intomainfrom
demo/05-pii-logging
Open

demo(05-pii-logging): 05 — PII logged in payment success path#154
EricCogen wants to merge 1 commit intomainfrom
demo/05-pii-logging

Conversation

@EricCogen
Copy link
Copy Markdown
Owner

05 — PII logged in payment success path

Expected verdict: ❌ Fails — GauntletCI should fire GCI0029 (PII in logs).

What changed

A new LogInformation call in OrderProcessor.ProcessAsync writes the
customer's email address and the raw charge amount to logs after a
successful payment, ostensibly "for the analytics rollout":

_logger.LogInformation(
    "Charged customer {Email} {Amount} {Currency} (order {OrderId})",
    order.Customer.Email, priced.Total.Amount, priced.Total.Currency, order.Id);

Why this is risky

  • Email addresses are personal data under GDPR/CCPA. Logs are routinely
    shipped to third-party aggregators (Datadog, Splunk, CloudWatch) that
    may not be in your data-processor agreement.
  • "Just for the rollout" log lines outlive their reason; they end up in
    six months of cold storage that nobody audits.
  • The amount + email pair is identifying enough to correlate users across
    systems.

What GauntletCI catches

GCI0029 PII in logs — a structured log argument named Email (or
matching email-like patterns) is being emitted to a logger.

See scenarios/05-pii-logging/README.md for the expected verdict.
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

GauntletCI found issues in this PR. See inline comments for details.

if (result.Success)
{
order.MarkPaid(_clock.UtcNow);
_logger.LogInformation("Charged customer email={Email} amount={Amount} currency={Currency} order={OrderId}", order.Customer.Email, priced.Total.Amount, priced.Total.Currency, order.Id);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GCI0029 — PII Entity Logging Leak

PII term 'email' found in log call — may expose sensitive data in src/OrderService/Processing/OrderProcessor.cs.

Evidence:

Line 50: _logger.LogInformation("Charged customer email={Email} amount={Amount} currency={Currency} order={OrderId}", order.Customer.Email, priced.Total.Amount, priced.Total.Currency, order.Id);

⚠️ Why it matters: Logging PII violates GDPR, CCPA, and HIPAA. Once in logs, PII propagates to log aggregators, storage, and third-party monitoring tools.

💡 Suggested action: Redact or omit PII from log calls. Log only anonymized identifiers (e.g. UserId, not Email or SSN).

Confidence: High | Severity: Warn

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8ca56fb543

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if (result.Success)
{
order.MarkPaid(_clock.UtcNow);
_logger.LogInformation("Charged customer email={Email} amount={Amount} currency={Currency} order={OrderId}", order.Customer.Email, priced.Total.Amount, priced.Total.Currency, order.Id);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Remove customer email from payment success logs

This new LogInformation call emits order.Customer.Email as a structured Email field, which stores personally identifiable information in centralized logs whenever a charge succeeds. That creates a concrete privacy/compliance risk (e.g., GDPR/CCPA exposure) and expands blast radius because log sinks are broadly accessible and retained longer than transactional data. Please avoid logging raw email here (use a non-PII identifier or a redacted/hashed value if correlation is required).

Useful? React with 👍 / 👎.

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