Skip to content

fix: reduce sensitive backend request logging#2186

Closed
JB-Bryant wants to merge 3 commits into
Cap-go:mainfrom
JB-Bryant:jb/log-redaction-metadata-only
Closed

fix: reduce sensitive backend request logging#2186
JB-Bryant wants to merge 3 commits into
Cap-go:mainfrom
JB-Bryant:jb/log-redaction-metadata-only

Conversation

@JB-Bryant
Copy link
Copy Markdown

@JB-Bryant JB-Bryant commented May 11, 2026

Summary

  • replace full request-body logging in billing/download/delete-failed-version endpoints with metadata-only summaries
  • avoid logging raw Stripe customer IDs from loaded org rows
  • preserve request flow and permission checks; this only reduces routine log retention

Security note

This is a public-safe log hardening change for the security retribution thread. It avoids retaining callback/success/cancel URLs, client reference IDs, attribution IDs, bundle names, and raw customer IDs in routine cloud logs where presence flags or stable IDs are enough for debugging.

Related: #1667
/claim #1667

Testing

  • git diff --check
  • bun run lint:backend could not be run locally because bun is not installed in this environment

Summary by CodeRabbit

  • Chores
    • Replaced raw request-body logging with structured, presence-flag style logs across several private backend endpoints to reduce exposed data in logs and improve observability.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0419b428-bd42-474f-9a56-25751f33541e

📥 Commits

Reviewing files that changed from the base of the PR and between 972243d and 36aab68.

📒 Files selected for processing (4)
  • supabase/functions/_backend/private/delete_failed_version.ts
  • supabase/functions/_backend/private/download_link.ts
  • supabase/functions/_backend/private/stripe_checkout.ts
  • supabase/functions/_backend/private/stripe_portal.ts
✅ Files skipped from review due to trivial changes (2)
  • supabase/functions/_backend/private/download_link.ts
  • supabase/functions/_backend/private/stripe_checkout.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • supabase/functions/_backend/private/delete_failed_version.ts
  • supabase/functions/_backend/private/stripe_portal.ts

📝 Walkthrough

Walkthrough

Four backend private handlers now log structured cloudlog events at entry (requestId plus presence flags and key request fields) instead of raw request bodies; intermediate auth/apikey logs were changed to presence booleans and Stripe handlers' post-DB logs now report presence of customer/callback state rather than full objects.

Changes

Structured Logging Across Backend Handlers

Layer / File(s) Summary
Entry-Point Request Logging
supabase/functions/_backend/private/delete_failed_version.ts, supabase/functions/_backend/private/download_link.ts, supabase/functions/_backend/private/stripe_checkout.ts, supabase/functions/_backend/private/stripe_portal.ts
Initial handlers now emit structured cloudlog entries containing requestId, resource identifiers, storage_provider/recurrence where applicable, and boolean presence flags (e.g., hasAppId, hasName, hasBundleId, isManifest) instead of logging full request bodies.
Auth / API Key Presence
supabase/functions/_backend/private/delete_failed_version.ts
apikey context logging now uses hasApikey/hasUser boolean fields instead of exposing apikeyId/userId.
Removed Auth Object Logs
supabase/functions/_backend/private/stripe_checkout.ts, supabase/functions/_backend/private/stripe_portal.ts
Intermediate cloudlog calls that previously logged the authenticated auth object were removed.
Post-Database-Load Logging
supabase/functions/_backend/private/stripe_checkout.ts, supabase/functions/_backend/private/stripe_portal.ts
After loading org data, handlers now log messages like stripe checkout org loaded / stripe portal org loaded with presence booleans (e.g., hasCustomer, hasCallbackUrl) instead of logging full org objects.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • Cap-go/capgo#2086: Applies the same structured request-id logging pattern to backend private endpoints (adds requestId and presence flags to cloudlog entries).

Poem

🐰 In hops of code I tidy logs,
I trade raw heaps for tidy clogs,
Request IDs and true/false signs,
Quiet notes, no noisy lines,
A rabbit's hop, small, neat, and bright.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: reduce sensitive backend request logging' accurately and specifically summarizes the main change: reducing sensitive data in backend request logs across multiple endpoints.
Description check ✅ Passed The description includes Summary and Security note sections addressing the key changes and objectives. However, it lacks proper Test plan details (only mentions git diff and notes bun unavailability) and is missing Screenshots section and completed Checklist as specified in the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@ThiagoCAltoe
Copy link
Copy Markdown

There is still one sensitive logging path left in the two billing endpoints touched here: both stripe_checkout.ts and stripe_portal.ts keep logging authContext.userId with message: 'auth' after the request-body/customer logs are redacted. That user id is still an authenticated user identifier tied to billing activity, so the PR reduces some leakage but does not fully remove the sensitive backend request logging for these flows.

Can you drop that cloudlog({ ..., message: 'auth', auth: authContext.userId }) line or replace it with a non-identifying boolean such as hasAuthContext: true?

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 11, 2026

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing JB-Bryant:jb/log-redaction-metadata-only (36aab68) with main (ee54a9c)2

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (cacf59d) during the generation of this report, so ee54a9c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@JB-Bryant
Copy link
Copy Markdown
Author

Thanks, agreed. I removed the two authContext.userId cloudlog calls from the billing endpoints and force-pushed the branch.

I also compacted the added metadata logs to avoid the Sonar duplication failure from the first revision. Waiting for checks to rerun now.

@ThiagoCAltoe
Copy link
Copy Markdown

Thanks for the quick update. I noticed one similar metadata path still remains in delete_failed_version.ts: the apikey context log still includes apikeyId and userId.

Since this PR is removing authenticated-user identifiers from backend request logs, could this log also be reduced to non-identifying metadata, e.g. hasApikey, hasUser, and mode, or dropped if it is no longer needed?

@JB-Bryant
Copy link
Copy Markdown
Author

Thanks, good catch. I updated that remaining apikey context log to keep only non-identifying metadata: hasApikey, hasUser, and mode.

Copy link
Copy Markdown

@mingisrookie mingisrookie left a comment

Choose a reason for hiding this comment

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

This still leaves raw request identifiers in the new "redacted" request logs, and several of them happen before the authorization check for that resource.

Examples from the patch:

  • delete_failed_version.ts logs app_id: body.app_id before checkPermission(..., { appId: body.app_id }).
  • download_link.ts logs app_id: body.app_id / bundleId: body.id before checkPermission(..., { appId: body.app_id }).
  • stripe_checkout.ts and stripe_portal.ts log raw orgId: body.orgId before checkPermission(..., { orgId: body.orgId }).

So the PR removes full-body logging, but an unauthorized caller can still cause arbitrary app/org identifiers to be written to operational logs. If the goal is metadata-only request logging for #1667, these should probably be hasAppId / hasBundleId / hasOrgId (and maybe type/field-count metadata) rather than the raw IDs. A small regression test around unauthorized requests would catch this difference, because the current functional path still uses the identifiers while the logs do not need to retain them.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@mingisrookie mingisrookie left a comment

Choose a reason for hiding this comment

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

Rechecked the latest commit (36aab683): the request-entry logs in the affected endpoints now use presence booleans (hasAppId, hasBundleId, hasOrgId) instead of raw app/org/bundle identifiers. I also ran ESLint on the touched files:

bun x eslint supabase/functions/_backend/private/delete_failed_version.ts supabase/functions/_backend/private/download_link.ts supabase/functions/_backend/private/stripe_checkout.ts supabase/functions/_backend/private/stripe_portal.ts

Result: passed with no reported issues.

throw simpleError('not_authorize', 'Not authorize')

cloudlog({ requestId: c.get('requestId'), message: 'user', org })
cloudlog({ requestId: c.get('requestId'), message: 'stripe checkout org loaded', hasCustomer: !!org.customer_id })

This comment was marked as spam.

@WcaleNieWolny
Copy link
Copy Markdown
Contributor

Closing as AI-generated spam. Part of a 50+ PR wave of duplicate redact logs PRs from disposable accounts. If you're human, open an issue first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants