Skip to content

fix(selfhost): route boot-time advisories through console.error so Sentry can see them - #6350

Merged
JSONbored merged 1 commit into
mainfrom
fix/public-origin-advisory-sentry-alert
Jul 16, 2026
Merged

fix(selfhost): route boot-time advisories through console.error so Sentry can see them#6350
JSONbored merged 1 commit into
mainfrom
fix/public-origin-advisory-sentry-alert

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • sqliteBackupAdvisory and publicOriginReachabilityAdvisory both log via console.warn with the explicit intent to "warn LOUDLY" — but installStructuredLogForwarding (wired inside initSentry) only intercepts console.log (forwarded only with an explicit level:error/fatal) and console.error (always forwarded); console.warn is never wrapped at all. Both advisories were therefore silently unreachable by Sentry regardless of whether Sentry was configured or the advisory condition was true.
  • Confirmed live (read-only SSH check): the self-hosted instance reviewing JSONbored's own repos has PUBLIC_API_ORIGIN set to a bare Tailscale (.ts.net) hostname, with no Funnel serve/funnel config enabled on that node (tailscale funnel status / tailscale serve status both report "No serve config") — genuinely, provably unreachable from GitHub's public image-fetching servers, not a false-positive Funnel case. publicOriginReachabilityAdvisory exists specifically to catch this (test(ui): add AGPL-3.0 badge to footer to validate visual-capture pipeline #4180), and PUBLIC_ORIGIN_ACKNOWLEDGED is unset on that box, so the advisory has been firing at every boot the whole time — just never reaching anyone, because of this bug.
  • Switches both advisories from console.warn to console.error. level: "warn" stays in the JSON payload, so this only changes which console method reaches the Sentry forwarder, not the reported severity (forwardStructuredLogToSentry still maps it to Sentry's "warning" level, not "error").
  • emptyConfigDirAdvisory has the identical bug but fires before initSentry in the boot sequence, so the same one-line fix doesn't help it — that needs the Sentry-init call moved earlier, a distinct and slightly riskier change. Flagged as a separate follow-up rather than scope-creeping into this PR.

The live PUBLIC_API_ORIGIN value itself is unchanged by this PR — fixing the actual misconfiguration (pointing it at a genuinely public origin, or enabling Funnel) is an infra decision for the operator, not a code change, and I don't have enough context on the intended DNS/reverse-proxy setup to make that call myself.

Advances #6325 — this PR only covers the alerting half (the advisory is now Sentry-visible). The issue stays open until the actual PUBLIC_API_ORIGIN value is corrected and a real PR's screenshot confirms "after" renders correctly, per the issue's own remaining deliverables.

Test plan

  • test/unit/selfhost-health.test.ts (the pure publicOriginReachabilityAdvisory/sqliteBackupAdvisory function tests, untouched by this wiring-only change) — 36 tests pass unchanged
  • test/unit/docs-selfhost-sentry-observability.test.ts — 4 tests pass unchanged
  • src/server.ts is Codecov-exempt (see codecov.yml's ignore: list), so this doesn't need new coverage
  • npm run typecheck clean
  • Full local npm run test:ci gate green

…ntry can see them

Advances #6325 -- covers the alerting half only; the issue stays open
until the live PUBLIC_API_ORIGIN misconfiguration itself is corrected.

sqliteBackupAdvisory and publicOriginReachabilityAdvisory both log via
console.warn with the explicit intent to "warn LOUDLY" -- but
installStructuredLogForwarding (wired inside initSentry) only intercepts
console.log (forwarded only with an explicit level:error/fatal) and
console.error (always forwarded); console.warn is never wrapped at all.
Both advisories were therefore silently unreachable by Sentry regardless
of whether Sentry was configured or the advisory condition was true.

Confirmed live: the self-hosted instance reviewing JSONbored's own repos
has PUBLIC_API_ORIGIN set to a bare Tailscale hostname
(edge-nl-01.raccoon-bushi.ts.net) with no Funnel serve/funnel config
enabled on that node (verified via `tailscale funnel status` / `tailscale
serve status`, both "No serve config") -- genuinely, provably unreachable
from GitHub's public image-fetching servers, not a false-positive Funnel
case. publicOriginReachabilityAdvisory exists specifically to catch this
(#4180), and PUBLIC_ORIGIN_ACKNOWLEDGED is unset on that
box, so the advisory has been firing at every boot the whole time --
just never reaching anyone, because of this bug.

Switches both advisories from console.warn to console.error. `level:
"warn"` stays in the JSON payload, so this only changes which console
method reaches the Sentry forwarder, not the reported severity
(forwardStructuredLogToSentry still maps it to Sentry's "warning" level,
not "error").

emptyConfigDirAdvisory has the identical bug but fires BEFORE initSentry
in the boot sequence, so the same one-line fix doesn't help it -- that
needs the Sentry-init call moved earlier, a distinct and slightly riskier
change, tracked separately rather than scope-creeping into this PR.

The live PUBLIC_API_ORIGIN value itself is unchanged by this PR -- fixing
the actual misconfiguration (pointing it at a genuinely public origin, or
enabling Funnel) is an infra decision for the operator, not a code change.
@JSONbored
JSONbored force-pushed the fix/public-origin-advisory-sentry-alert branch from f5b3bb7 to c345711 Compare July 16, 2026 03:34
@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 16, 2026
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-16 04:02:39 UTC

1 file · 1 AI reviewer · 2 blockers · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
The diff swaps console.warn for console.error at two call sites in src/server.ts so that sqliteBackupAdvisory and publicOriginReachabilityAdvisory can be picked up by installStructuredLogForwarding, which per the shown sentry.ts context only intercepts console.log (error/fatal only) and console.error. The change is minimal and well-scoped (two lines + comments), and the PR description gives a concrete, verified production trigger (PUBLIC_API_ORIGIN pointed at an unreachable Tailscale hostname) rather than a hypothetical one. The one open question is whether forwardStructuredLogToSentry's own severity-threshold check (default 'error', and these payloads still carry level:'warn'/'warning') would filter these out downstream even after being intercepted — the doc comment's claim that console.error is 'always forwarded' regardless of payload level isn't verifiable from the excerpts shown.

Nits — 5 non-blocking
  • src/server.ts: no test exercises that these two advisories now reach the Sentry forwarder (or are filtered by it) — worth a unit test around installStructuredLogForwarding/forwardStructuredLogToSentry with a level:'warn' payload sent via console.error to lock in the intended behavior, especially since the shown resolveSentryMinSeverity default is 'error' and it's not obvious from this diff alone that a 'warning'-level payload clears that bar even via the console.error path.
  • src/server.ts: the inline comments on both call sites are quite long (5-6 lines each) duplicating most of the PR description — consider trimming to a one-line pointer plus the issue number, since the full rationale already lives in the PR/issue history.
  • Confirm (and ideally assert via a test) that forwardStructuredLogToSentry actually forwards a level:'warn' JSON payload delivered through console.error under the default SENTRY_MIN_SEVERITY='error' threshold — if it doesn't, this fix silently doesn't achieve its stated goal for default-configured instances.
  • Since emptyConfigDirAdvisory has the identical bug but isn't fixed here, consider filing/linking the follow-up issue now (if not already) so it doesn't get lost, as the PR description promises.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

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.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.

2. Maintainer requires a linked issue — Link the relevant issue (for example `Closes #123`) before opening the PR.

Decision drivers

  • ❌ Code review — 2 blockers (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
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: 42 registered-repo PR(s), 34 merged, 408 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 42 PR(s), 408 issue(s).
Improvement ℹ️ None detected risk: low · value: none
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 42 PR(s), 408 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Then work through the remaining 2 steps in the Signals table above.
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.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 16, 2026
@JSONbored
JSONbored merged commit df3045c into main Jul 16, 2026
16 checks passed
@JSONbored
JSONbored deleted the fix/public-origin-advisory-sentry-alert branch July 16, 2026 04:06
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.60%. Comparing base (03a43e9) to head (c345711).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6350   +/-   ##
=======================================
  Coverage   95.60%   95.60%           
=======================================
  Files         597      597           
  Lines       47191    47191           
  Branches    15019    15019           
=======================================
  Hits        45117    45117           
  Misses       1290     1290           
  Partials      784      784           
Flag Coverage Δ
shard-1 43.98% <ø> (-0.18%) ⬇️
shard-2 36.81% <ø> (+0.41%) ⬆️
shard-3 32.45% <ø> (-0.09%) ⬇️
shard-4 34.57% <ø> (-0.06%) ⬇️
shard-5 30.98% <ø> (-0.60%) ⬇️
shard-6 45.31% <ø> (+0.44%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant