Skip to content

fix(webhook): omit repo names from enqueue error logs - #3614

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-webhook-enqueue-errors-privacy-leak
Jul 5, 2026
Merged

fix(webhook): omit repo names from enqueue error logs#3614
JSONbored merged 1 commit into
mainfrom
codex/fix-webhook-enqueue-errors-privacy-leak

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent private repository full names from being forwarded to Sentry tags/titles by removing repository from structured enqueue-failure logs so self-host telemetry does not leak repo metadata.

Description

  • Remove repository: eventRow.repositoryFullName from the missing-WEBHOOKS-binding and queue-send failure console.error JSON payloads in enqueueWebhookByEnv and update the corresponding unit tests to assert repository names are not emitted.

Testing

  • Ran npx vitest run test/unit/webhook.test.ts --reporter=verbose and the webhook unit tests passed (1 file, 20 tests all green).
  • Attempted the full local gate via git diff --check && npm run test:ci, but the run did not complete here due to long-running suites and existing unrelated timeouts / failures observed in test/unit/queue.test.ts and test/unit/backfill.test.ts, so the end-to-end CI gate could not be finalized in this environment.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 5, 2026
@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-05 19:47:38 UTC

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This diff removes `repository: eventRow.repositoryFullName` from the two enqueue-failure `console.error` JSON payloads in `enqueueWebhookByEnv` (missing-binding and queue-send-failure paths) so repo full names no longer reach Sentry via these structured logs, and updates the three corresponding assertions in webhook.test.ts to check `!line.includes("JSONbored/gittensory")` / `not.toContain`. The change is narrow, internally consistent (code and tests move together), and the removed field was purely diagnostic metadata, not used for control flow, so no behavior other than log content changes. CI is green across all checks including codecov/patch.

Nits — 4 non-blocking
  • The updated comment at src/github/webhook.ts (missing-binding branch) says repository/installation 'stay out of the forwarded log,' but installationId was never included in this console.error payload before or after the diff, so the comment slightly overstates what changed.
  • The PR description doesn't reference an open issue/tracker number this fix closes, which this repo's contribution convention expects for external PRs even for small fixes like this.
  • Consider also confirming (e.g. via a quick grep) that no other selfhost log line in this file still emits `repositoryFullName` directly, since the fix here only touches these two call sites.
  • If Sentry tagging on `repository` is the actual leak vector, it may be worth a one-line note on whether other structured logs elsewhere in the codebase have the same pattern, to scope whether this is a one-off fix or the first of several.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 56 registered-repo PR(s), 46 merged, 493 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 56 PR(s), 493 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 56 PR(s), 493 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.11%. Comparing base (7408da9) to head (98d562a).
⚠️ Report is 25 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3614   +/-   ##
=======================================
  Coverage   93.11%   93.11%           
=======================================
  Files         305      305           
  Lines       31532    31532           
  Branches    11509    11509           
=======================================
  Hits        29361    29361           
  Misses       1517     1517           
  Partials      654      654           
Files with missing lines Coverage Δ
src/github/webhook.ts 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 5, 2026
@JSONbored
JSONbored merged commit 20e04c5 into main Jul 5, 2026
10 checks passed
@JSONbored
JSONbored deleted the codex/fix-webhook-enqueue-errors-privacy-leak branch July 5, 2026 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

1 participant