Route the NeoWiki log channel, and keep credentials out of it - #1258
Open
alistair3149 wants to merge 2 commits into
Open
Route the NeoWiki log channel, and keep credentials out of it#1258alistair3149 wants to merge 2 commits into
alistair3149 wants to merge 2 commits into
Conversation
A graph backend reports what it could not reach by quoting the request it made, so the Neo4j client's connection failure carries the bolt URI userinfo and all. Three log sites relayed `getMessage()` verbatim while eleven others already passed it through `BackendFailureMessage::withoutCredentials()`, so a store's password reached the log on every save for as long as the backend was down — the outage during which an operator is most likely to be reading it. Redaction keeps what names the server, so the diagnostic still says which endpoint was unreachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MediaWiki routes no log channel anywhere by default, so every diagnostic NeoWiki emits was discarded on a stock install, including the bundled one. Several pages already told readers that "the NeoWiki channel says which pages failed and why"; none of them said how to make it say anything. The stack now routes the channel to container stderr, at every level: a rebuild records a skipped page at info, and that is a store left stale. Volume is bounded by log rotation on the service rather than by dropping entries, since a stack whose EDM projection has no Mapping page reports a failure on every save. `installation.md` gains the section that documents this, and becomes the one home for it: the pages that mention the channel now link there instead of restating it. The graph store name rules go the same way — the log names the rule and the fix when a name is refused, so `extending.md` states what a refused name costs and points at the log for the rest, rather than enumerating conditions that were incomplete anyway (the reserved set is wiki configuration, not just `neo4j`). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows-up to #1250
@JeroenDeDauw's review of #1250 called the graph store name paragraph in
extending.mdbadly worded clutter, and saidone sentence was all it needed. The question that raised: those rules are already in the code's log messages, so does
the doc need to state them at all?
It does today, because nothing is listening. NeoWiki logs every diagnostic through
LoggerFactory::getInstance( 'NeoWiki' ), and MediaWiki routes no channel anywhere by default. With no destination forthe channel and no
$wgDebugLogFile,LegacyLogger::__construct()setsminimumLeveltoLEVEL_INFINITY, solog()returns at its first guard — before the emit path, before
MW_LOG_STDERR, and beforeMWDebug::debugMsg(), which iswhat a
$wgDebugCommentspage would otherwise have rendered. Measured on a fresh stack: registering a store asneo4jproduced no warning in
docker logs, an empty<!-- Debug output: -->block in the page HTML, no store, and no clue.An operator was not blind:
RebuildGraphDatabases.phpprints per-store status and a failed-page count and exitsnon-zero, and
Special:GraphStoresshows sync state and counts. What lived on the channel alone was every reason —the script says as much, reporting the failing page ids and then deferring with
'. The NeoWiki log channel says why.'Three categories had no other surface at all: a refused store name, an RDF projection drop, and a failed save. The last
is the worst, because
GraphStoreStatusLookup::stateOf()derives In sync / Stale from a Mapping redefinition againstthe last successful run, so a store that rotted because Neo4j was down during edits keeps reading In sync.
This also applies to
maintenance.md, which twice pointed readers at this channel for rebuild diagnostics, and tordf-export.mdandontology-mapping.md, which each promised something "is logged" without naming where.Keeping credentials out of it
Routing the channel would have made a latent leak live, which is why fixing it is the earlier commit.
A graph backend reports what it could not reach by quoting the request it made, so the Neo4j client's failure carries
the bolt URI userinfo and all. Three log sites relayed
getMessage()verbatim while eleven others already passed itthrough
BackendFailureMessage::withoutCredentials(). Reproduced against a live stack with an unreachable host:On a default install this was latent, for the reason above. It was already live on any wiki that had set
$wgDebugLogGroups['NeoWiki'],$wgDebugLogFile,$wgShowDebugor$wgDebugToolbar— that is, on exactly the wikisthat had taken the advice this PR now writes down. It fires on every page save for as long as the backend is down, the
outage during which an operator is most likely to be reading the log.
After the fix the same probe yields
bolt://unreachable-host:7687: the password is gone, the unreachable server isstill named.
Routing
The bundled stack now sends the channel to container stderr, at every level. No level floor in either mode:
make demoand
make uprun production mode, so awarningfloor would have applied to the documented install and dropped therebuild's skipped-page entries — which
logSkippedPageexists to record, because a skipped page is a store left stale.Volume is bounded by log rotation on the service instead, since a stack whose EDM projection has no Mapping page reports
a failure on every save and no compose file set a
logging:key.Docs
installation.mdgains the section documenting this and becomes the one home for it; the pages that mention the channellink there rather than restating it.
extending.mddrops the enumeration, keeps what a refused name costs, and pointsat the log for the rest.
Dropping the enumeration is also a small accuracy gain: it claimed
neo4jwas the reserved name, butgetBundledStoreNames()reserves every configured SPARQL store name too, which an extension developer cannot know inadvance. The reserved set is wiki configuration and was never statable in a doc.
Considered, omitted
and the fix when you do. Restoring it means starting to rebuild the enumeration; say the word and it goes back as four
words in the existing sentence.
Special:GraphStores, which would make them visible with no logging config atall. That is a feature change, not a docs fix.
$e->getMessage()in 500 responses — the same raw-message pattern over HTTP rather than toa log. Untouched by this change and wants its own audit.
Verification
make csclean.DocsPathReferencesTest(791 assertions with the plugin suite),OnRevisionCreatedHandlerTestandExportPageRdfApiTestgreen. The credential regression test was confirmed failing against the unredacted code beforethe fix. Routing and redaction were both verified end to end on a live stack, over CLI stderr and the web path into
docker logs, and the pre-change silence was confirmed by unsetting the routing and re-running the same request.make logswas confirmed to attach to a stack brought up without the dev compose file.AI-authored — Claude Code,
Opus 5 (1M context); investigation, implementation and verification in-session; reviewed by three independent agents whose findings are folded in; not yet human-reviewed.