Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions dist/index.js

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion docs/how-it-works.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ External links are checked with browser-like headers to avoid bot-detection bloc

When an external link redirects, HyperHawk follows the redirect chain and suggests updating to the final destination URL, but only when the redirect stays on the same hostname. Cross-domain redirects (typically auth/login flows) are not suggested as corrections.

Tracking parameters that the redirect adds (`visit_id`, `rd`, `utm_*`, ad click ids, and similar) are stripped from the suggestion, because they identify the visit rather than the content and go stale as soon as they are committed. A parameter that was already on your link is kept. If stripping leaves the destination identical to the original link, no suggestion is made at all.

### GitHub user-attachment assets

URLs matching `https://github.com/user-attachments/assets/...` are treated as always valid without making an HTTP request. These are image and file uploads attached to issues, PRs, and discussions. They cannot be deleted without GitHub admin intervention and are effectively permanent, so verifying them would add complexity for no benefit.
Expand Down Expand Up @@ -74,7 +76,7 @@ Full GitHub URLs that point back to the current repository (e.g. `https://github

### Redirect suggestions

External links that redirect to a different URL on the same host get a suggestion to update to the final destination. Fragment identifiers (`#section`) from the original URL are preserved when the redirect target does not include one.
External links that redirect to a different URL on the same host get a suggestion to update to the final destination, with any tracking parameters the redirect added stripped out. Fragment identifiers (`#section`) from the original URL are preserved when the redirect target does not include one.

## Permissions

Expand Down
74 changes: 71 additions & 3 deletions src/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -613,19 +613,80 @@ async function checkSameOrg(link: LinkInfo, octokit: Octokit, config: Config): P
}
}

/**
* Query parameters a server attaches to identify the visit rather than the
* content. support.google.com, for example, redirects to a URL carrying a
* `visit_id` session token that is stale the moment it is written down.
* Committing one of these into a markdown file is never what the author wants,
* so they are dropped from redirect suggestions. Anything prefixed `utm_` is
* treated the same way.
*/
const TRACKING_PARAMS = new Set([
'visit_id', // support.google.com session token
'rd', // support.google.com redirect counter
'sjid', // Google session join id
'gclid', 'dclid', 'fbclid', 'msclkid', // ad platform click ids
'mc_cid', 'mc_eid', // Mailchimp campaign and email ids
'_ga', '_gl', // Google Analytics cross-domain linker
]);

function isTrackingParam(name: string): boolean {
return TRACKING_PARAMS.has(name) || name.startsWith('utm_');
}

/**
* Drop tracking parameters that the redirect introduced. A parameter already
* present on the original link is left alone: the author put it there
* deliberately, and this strips server noise, it does not rewrite intent.
*/
function stripTrackingParams(finalUrl: string, originalUrl: string): string {
let parsed: URL;
let original: URL;
try {
parsed = new URL(finalUrl);
original = new URL(originalUrl);
} catch {
return finalUrl;
}

const drop = [...parsed.searchParams.keys()].filter(
name => isTrackingParam(name) && !original.searchParams.has(name)
);
if (drop.length === 0) return finalUrl;

for (const name of drop) {
parsed.searchParams.delete(name);
}
return parsed.toString();
}

/**
* Returns true if two URLs are the same once parsed, so that a redirect which
* only added tracking noise is not mistaken for a content move.
*/
function isSameUrl(urlA: string, urlB: string): boolean {
if (urlA === urlB) return true;
try {
return new URL(urlA).toString() === new URL(urlB).toString();
} catch {
return false;
}
}

/**
* Build the final redirect URL, re-attaching the fragment from the original URL
* when the redirect target does not include one (servers typically strip
* fragments from Location headers).
*/
function buildRedirectUrl(originalUrl: string, finalUrl: string): string {
const cleanedUrl = stripTrackingParams(finalUrl, originalUrl);
const originalHash = originalUrl.indexOf('#') >= 0 ? originalUrl.slice(originalUrl.indexOf('#')) : '';
if (!originalHash) return finalUrl;
if (!originalHash) return cleanedUrl;

// If the final URL already has a fragment, keep it as-is
if (finalUrl.indexOf('#') >= 0) return finalUrl;
if (cleanedUrl.indexOf('#') >= 0) return cleanedUrl;

return finalUrl + originalHash;
return cleanedUrl + originalHash;
}

/**
Expand Down Expand Up @@ -702,6 +763,13 @@ async function checkExternal(link: LinkInfo, config: Config): Promise<CheckResul
// Cross-domain redirects are typically auth/login flows, not content moves.
if (isSameHost(link.url, result.finalUrl)) {
const correctedUrl = buildRedirectUrl(link.url, result.finalUrl);
// The redirect only added tracking noise, so there is nothing to fix.
if (isSameUrl(correctedUrl, link.url)) {
core.debug(`[external] ${link.url} redirected only to add tracking parameters - skipping suggestion`);
const r = { ok: true, statusCode: result.status };
resultCache.set(link.url, r);
return { link, ...r };
}
const suggestion = link.lineContent.trimEnd().replace(link.url, () => correctedUrl);
const r = { ok: true, statusCode: result.status, correctedUrl, suggestionOnly: true };
resultCache.set(link.url, r);
Expand Down
5 changes: 5 additions & 0 deletions tests/expected-output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
::debug::[external] <server>/not-found returned HTTP 404 for HEAD - retrying with GET
::debug::[external] <server>/head-404-get-200 returned HTTP 404 for HEAD - retrying with GET
::debug::[external] <server>/head-404-get-redirect returned HTTP 404 for HEAD - retrying with GET
::debug::[external] <server>/tracking-only redirected only to add tracking parameters - skipping suggestion
::debug::[external] https://github.com/user-attachments/assets/35163316-65df-4f8d-bf85-03650025a7b4 is a GitHub user-attachment asset, treating as valid
tests/test-document.md:1001 | <server>/redirect-301 | suggestion -> <server>/final
tests/test-document.md:1002 | <server>/redirect-302 | suggestion -> <server>/final
Expand All @@ -28,6 +29,10 @@ tests/test-document.md:1010 | <server>/not-found | broken | HTTP 404
tests/test-document.md:1011 | https://github.com/user-attachments/assets/35163316-65df-4f8d-bf85-03650025a7b4 | ok
tests/test-document.md:1012 | <server>/head-404-get-200 | ok
tests/test-document.md:1013 | <server>/head-404-get-redirect | suggestion -> <server>/final
tests/test-document.md:1014 | <server>/tracked-move | suggestion -> <server>/final
tests/test-document.md:1015 | <server>/tracking-only | ok
tests/test-document.md:1016 | <server>/keeps-author-param?utm_source=author | suggestion -> <server>/final?utm_source=author
tests/test-document.md:1017 | <server>/tracked-move#my-anchor | suggestion -> <server>/final#my-anchor
tests/test-document.md:11 | ../release.ps | broken | File not found: <root>/release.ps
tests/test-document.md:13 | ../tsconfig.jso | broken | File not found: <root>/tsconfig.jso
tests/test-document.md:15 | ../action.ym | broken | File not found: <root>/action.ym
Expand Down
27 changes: 26 additions & 1 deletion tests/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function startRedirectServer(): Promise<{ baseUrl: string; close: () => Promise<
} else if (url === '/redirect-301-step2') {
res.writeHead(301, { Location: '/final' });
res.end();
} else if (url === '/final') {
} else if (url === '/final' || url.startsWith('/final?')) {
res.writeHead(200);
res.end('OK');

Expand Down Expand Up @@ -79,6 +79,27 @@ function startRedirectServer(): Promise<{ baseUrl: string; close: () => Promise<
res.writeHead(302, { Location: `http://localhost:${addr.port}/final` });
res.end();

// Redirect that moves the page AND appends tracking params: the suggestion
// should point at the new path with the tracking params stripped
} else if (url.startsWith('/tracked-move')) {
res.writeHead(301, { Location: '/final?visit_id=639222257369492970-241975759&rd=2&utm_source=redirect' });
res.end();

// Redirect that only appends tracking params to the same path: once they
// are stripped there is nothing left to suggest
} else if (url === '/tracking-only') {
res.writeHead(301, { Location: '/tracking-only?visit_id=abc123&rd=1' });
res.end();
} else if (url.startsWith('/tracking-only?')) {
res.writeHead(200);
res.end('OK');

// Redirect that keeps a param the author already had, alongside a new
// tracking param: the author's param must survive
} else if (url.startsWith('/keeps-author-param')) {
res.writeHead(301, { Location: '/final?utm_source=author&visit_id=xyz789' });
res.end();

// Errors on HEAD but serves the page on GET (support.google.com behaviour)
} else if (url === '/head-404-get-200') {
if (req.method === 'HEAD') {
Expand Down Expand Up @@ -142,6 +163,10 @@ function buildExternalLinks(baseUrl: string, testFile: string): LinkInfo[] {
{ url: `${baseUrl}/not-found`, text: 'plain 404', line: 1010, lineContent: `[plain 404](${baseUrl}/not-found)` },
{ url: `${baseUrl}/head-404-get-200`, text: 'head 404 get 200', line: 1012, lineContent: `[head 404 get 200](${baseUrl}/head-404-get-200)` },
{ url: `${baseUrl}/head-404-get-redirect`, text: 'head 404 get redirect', line: 1013, lineContent: `[head 404 get redirect](${baseUrl}/head-404-get-redirect)` },
{ url: `${baseUrl}/tracked-move`, text: 'tracked move', line: 1014, lineContent: `[tracked move](${baseUrl}/tracked-move)` },
{ url: `${baseUrl}/tracking-only`, text: 'tracking only', line: 1015, lineContent: `[tracking only](${baseUrl}/tracking-only)` },
{ url: `${baseUrl}/keeps-author-param?utm_source=author`, text: 'keeps author param', line: 1016, lineContent: `[keeps author param](${baseUrl}/keeps-author-param?utm_source=author)` },
{ url: `${baseUrl}/tracked-move#my-anchor`, text: 'tracked move with fragment', line: 1017, lineContent: `[tracked move with fragment](${baseUrl}/tracked-move#my-anchor)` },
{ url: 'https://github.com/user-attachments/assets/35163316-65df-4f8d-bf85-03650025a7b4', text: 'user attachment', line: 1011, lineContent: '[user attachment](https://github.com/user-attachments/assets/35163316-65df-4f8d-bf85-03650025a7b4)' },
];

Expand Down
Loading