Repository navigation
port(upstream#1893): drop hardcoded og:url so shared links stay on this instance - #30
Merged
Merged
Conversation
… on the instance (Kpa-clawbot#1893) Fixes Kpa-clawbot#1890. ## The problem `public/index.html:16` shipped this to every deployment: ```html <meta property="og:url" content="https://analyzer.00id.net"> ``` Open Graph consumers — Facebook and Messenger among them — treat `og:url` as the canonical destination. Clicking the preview of a link shared from *any* CoreScope instance navigated to that one host. The direct link text still resolved correctly, which is why this went unnoticed; the preview card and the surrounding message body did not. It is the only occurrence in the frontend. ## The change Remove the tag. `og:url` is optional — with no tag present, consumers fall back to the URL they crawled, which is correct for every deployment and needs no configuration. ## Why not the config-driven variant The issue also proposes deriving the URL from `config.json`. I did not take that shape, on purpose: `index.html` is pre-processed **once at startup** — `spaHandler` reads it and substitutes `__BUST__` (`cmd/server/main.go:565`), then serves the same byte slice for every request. A correct per-host `og:url` therefore needs either a new public-URL config key or per-request templating of the index. Both are decisions about config surface and request-path cost that belong to you, and neither is needed to stop the redirect. Happy to follow up with whichever shape you prefer — this PR is the part that is unambiguous. ## What is left alone `og:image` still points at `raw.githubusercontent.com/Kpa-clawbot/corescope/master/public/og-image.png`. That is the project's own asset, a shared project resource rather than a redirect target, so it is correct for every instance to reference it. ## Test `test-issue-1890-og-url.js`, a static scan, registered in `test-all.sh`: - no `og:url` meta tag - no `rel="canonical"` link - no `00id.net` reference anywhere in `index.html` - `og:title` / `og:description` / `og:image` still present That last assertion is deliberate: without it the guard could be satisfied by deleting the whole embed block. Watched fail first — 2 passed, 2 failed before the change, 4 passed after. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit c5a71b3) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rl comments Two review fixes on this PR. 1. CI registration. `test-issue-1890-og-url.js` was only wired into `test-all.sh` (used by `npm test`), not into the JS test step in `.github/workflows/deploy.yml`. CI could not catch a regression of the hardcoded og:url. Added `node test-issue-1890-og-url.js` to that step's existing list, directly before `test-issue-1375-scope-stats- fetch.js` (the test that currently stops the step). test-all.sh is unchanged; the registration there was already correct. 2. Comment accuracy, in `public/index.html` and `test-issue-1890-og-url.js`. The removed tag declared the upstream analyzer instance's own URL as the canonical URL for every self-hosted deployment (Kpa-clawbot#1890) -- correct metadata for upstream, wrong for everyone else. Reworded both comments to say that precisely, and to stop implying things not established: - og:url is metadata, not an HTTP redirect, and does not by itself decide what a viewer's click navigates to. - Per the Open Graph protocol it is a required property, not "optional" -- omitting it leaves the crawled URL as the fallback canonical reference, which is what actually fixes this for every instance. - This change does not refresh previews a consumer has already cached under the old, hardcoded value. No functional assertions changed in test-issue-1890-og-url.js -- only the file-level comment. The `public/index.html` comment change had to avoid writing the literal removed domain: the test's own 4th assertion scans the whole file for that string, and an earlier draft of this comment briefly reintroduced it and failed its own guard before landing on the current wording. Verified on this branch's own base (pre-#51/#33 master) and against a merge into current master: test-issue-1890-og-url.js passes 4/4 on the resulting index.html and still fails 2/4 (og:url present, 00id.net present) against the original hardcoded tag. The merge result's deploy.yml differs from current master by exactly the one added test line; release-fast-path.yml and cmd/server/fork_guard_workflow_test.go are byte-identical to master, and all four #51 repository guards (the five build-and-publish publish steps, release-artifacts, deploy, publish, retag-or-fallback) are present in the merged file. Co-Authored-By: Claude Sonnet 5 <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.
Split out of #25 (commit
4be0c596there). This branch holds exactly one upstream change so it can be reviewed, tested and reverted on its own.Upstream
c5a71b34ec948f69c17956c4deebffe7e8a5051egit cherry-pick -xonto masterfda24ca5; upstream authorship kept, and the commit message carries the(cherry picked from commit …)line.public/index.html, newtest-issue-1890-og-url.js(registered intest-all.sh).Problem
index.htmlshipped<meta property="og:url" content="https://analyzer.00id.net">. Open Graph consumers (Facebook, Messenger, …) treatog:urlas the canonical destination, so link previews shared from our instance navigated to analyzer.00id.net.Change
Removes the hardcoded
og:urland leaves a comment explaining why. Without it, consumers use the URL they crawled, which is correct for every deployment without configuration.Adaptation to this fork
None. The cherry-pick applied without conflicts and the changed lines are identical to upstream.
Dependencies and merge order
fda24ca5and needs no other PR from this split.TestPruneOldNeighborMetricsdeterministic). If test(ingestor): make neighbor metrics pruning deterministic #33 lands first, the expected CI failure named below disappears; nothing in this PR depends on it.Verification
Local run of the same commands as CI's “Go Build & Test” job (server tests with
-race), on this branch and on masterfda24ca5under the same conditions (same machine, run one after another):fda24ca5xss-gate-diffchannel-lib-testdecrypt-cli-build-testdockerfile-copy-invariantsdeclare -A), macOS has 3.2; identical on masterstaging-disk-monitorcss-vars-linttest-issue-1375-scope-stats-fetch.jsExactly oneapi('/scope-stats'call exists (the fixed loader) — found 2test-issue-1648-m4-emoji-scan.jsmap.js has 1 emoji/misc-icon hit(s):test-a11y-axe-routes-coverage.jsaxe ROUTES missing analytics tabs (issue #1706): areas, foreign-traffic, wardrivingtest-frontend-helpers.jsfavStar returns empty star for non-favorite: The expression evaluated to a falsy value:,favStar returns filled star for favorite: The expression evaluated to a falsy value:Baseline failures (fail identically on master; not introduced or changed here): see rows marked baseline failure, unchanged.
Browser validation (local, fixture DB, no staging/production): Local Go server (built from master) on the committed E2E fixture DB (freshened, migrated and seeded exactly like CI), serving this branch's
public/, compared side by side with the same server serving master'spublic/. Served HTML and live DOM compared on the same fixture: masterGET /contains<meta property="og:url" content="https://analyzer.00id.net">anddocument.querySelector('meta[property="og:url"]').contentis that host. this branch serves the explanatory comment instead; noog:urlelement in the DOM,og:title/og:image/og:typestill present, home page renders normally (29 nav links).Not run:
eslint(not installed locally; CI installs it on the fly).-race/tests for modules this PR does not touch (unchanged code, identical to master).Expected GitHub CI: “Go Build & Test” is expected to fail on
TestPruneOldNeighborMetrics, which already fails on master (see #25's run). Downstream jobs (Playwright, image build) are therefore skipped. “Deploy Staging” and all GHCR publish steps only run onpushtomasterand cannot run for this PR.Two further ingestor tests have failed intermittently in this split's CI on branches whose
cmd/ingestortree is byte-identical to master (#27, #28), so they can also appear here without being caused by this change:TestBackfillTxLastSeen_ResolvesFromMaxObservationTimestamp: also reproduced locally on unmodified master.TestMQTTStallWatchdog_DisconnectedEscalationThrottled_1749: the suite flake that upstream test(ingestor): join the watchdog loop goroutine instead of only asking it to stop Kpa-clawbot/CoreScope#2003 (also split out of port(upstream): 26 clean upstream fixes — prune batching, /ws limits, observer liveness, watchdog race #25) addresses.GitHub CI result: run 34749778310 on
dcf87c79. Go Build & Test: failure; all downstream jobs incl. Deploy Staging skipped. Failed tests:TestPruneOldNeighborMetrics: fails on master, documented baselineTestBackfillTxLastSeen_ResolvesFromMaxObservationTimestamp: intermittent, reproduced on unmodified master locallyReview fix (
dcf87c79→efa6a033)Two minimal review fixes, one commit, on this same branch:
CI registration.
test-issue-1890-og-url.jswas registered intest-all.sh(npm test) but not in the JS test step of.github/workflows/deploy.yml, so CI could not catch a regression of the hardcodedog:url. Addednode test-issue-1890-og-url.jsto that step's existing list, directly beforetest-issue-1375-scope-stats-fetch.js(the test that currently stops the step first).test-all.shis unchanged..github/workflows/deploy.ymlhad none of those guards. The fix was made as a minimal one-line addition on this branch's existing file, not by restoring or overwriting it with master's guarded version.6b70e94a) without touching any checkout: the mergeddeploy.ymldiffers from master by exactly that one added line;release-fast-path.ymlandcmd/server/fork_guard_workflow_test.gocome out byte-identical to master; and all fourgithub.repository == 'Kpa-clawbot/CoreScope'guards from ci: guard publish, release, deploy and badge jobs to the upstream repository #51 (the fivebuild-and-publishpublish steps,release-artifacts,deploy,publish,retag-or-fallback) are present in the merged file, assuming every test passes.Comment accuracy, in
public/index.htmlandtest-issue-1890-og-url.js(comments only — no assertions changed). The removed tag declared the upstream analyzer instance's own URL as the canonical URL for every self-hosted deployment (index.html drives Facebook /Facebook Messager traffic to https://analyzer.00id.net Kpa-clawbot/CoreScope#1890) — correct metadata for upstream, wrong for anyone else. Reworded both comments to say precisely that, and to drop claims the sources don't support:og:urlis metadata, not an HTTP redirect, and does not by itself decide what a viewer's click navigates to.public/index.htmlcomment had to describe this without writing the literal removed domain: the test's own 4th assertion scans the whole file for that string, and an earlier draft briefly reintroduced it and failed its own guard.Local verification on the new head:
test-issue-1890-og-url.json this branch's ownindex.htmlog:urlindex.htmlog:urltag and the00id.netscan, as before)node --check test-issue-1890-og-url.jsruby -ryamlparse of.github/workflows/deploy.ymlgit diff --check(new commit vs old head)index.html(test-channel-decrypt-insecure-context.js,test-channel-issue-1087.js,test-issue-1361-cb-presets.js,test-issue-1380-cb-sim-overlay.js,test-issue-1473-prefix-generator.js,test-issue-1473-reserved-prefixes.js,test-issue-1648-m1-emoji-scan.js,test-nav-dynamic-link-lifecycle.js,test-nodes-export-wiring.js,test-payload-labels-namespace.js)cmd/serverTestSpaHandler,TestSpaHandlerCacheBust,TestSpaHandlerPathTraversal,TestStaticAssetsDoNotEmitBareNoStore,TestWsOrStaticNonWebSocket,TestAPIRoutesEmitNoStoreCacheControlDiff scope of the reviewfix commit:
.github/workflows/deploy.yml(+1 line),public/index.html(comment only),test-issue-1890-og-url.js(file-level comment only).test-all.shuntouched.Push safety: this branch is not
masterand no tag was pushed, so the push could only re-triggerCI/CD Pipelineonpull_request: synchronizefor this PR — never thepush-gated GHCR/release/deploy/badge jobs, all of which additionally require theKpa-clawbot/CoreScoperepository guard from #51 regardless.Squad Heartbeattriggers only onpull_request: closed, which this push is not.GitHub CI on the new head: pending at the time of this update — see the latest run on this PR for its actual result. The "GitHub CI result" section above describes the run on the previous head (
dcf87c79) and is left as a historical record.🤖 Generated with Claude Code