Repository navigation
fix: generate asset permalinks on the canonical Commons host - #1003
Conversation
|
Severity: High — books.ts:2327: Removing Severity: High — books.ts:2109: The unauthenticated TOC endpoint now returns the unfiltered page tree, exposing titles, IDs, and URLs of Private and Semi-Private pages. Restore restriction filtering for public requests. Severity: Medium — projectfiles.ts:689: |
| { host: receivingHost(req) }, | ||
| "redirecting permalink to canonical host" | ||
| ); | ||
| return res.redirect(301, target); |
There was a problem hiding this comment.
I know we originally spec'd 301 here, but let's switch to 302. This will be safer and help prevent Cloudflare and browsers caching a bad redirect indefinitely if something goes wrong. Also, let's add .setHeader("Cache-Control", "no-store") before .redirect for the same reason
| const withProtocol = /^https?:\/\//i.test(raw) ? raw : `https://${raw}`; | ||
| try { | ||
| return new URL(withProtocol).origin; | ||
| } catch { |
There was a problem hiding this comment.
Let's add logger.warn({ configured }, "Invalid COMMONS_CANONICAL_DOMAIN, falling back to default"); for extra visbility
| if (!isCanonicalPermalinkHost(req)) { | ||
| const queryIndex = req.originalUrl.indexOf("?"); | ||
| const query = queryIndex === -1 ? "" : req.originalUrl.slice(queryIndex); | ||
| const target = `${getCommonsCanonicalURL()}/permalink/${projectID}/${fileID}${query}`; |
There was a problem hiding this comment.
Let's add
if (target.endsWith(req.originalUrl)) {
permalinkLog.warn({ host: receivingHost(req) }, "canonical host check rejected its own origin");
} else {
[rest of current code]
}
here. This is a cheap guard that will help us see if a mismatch on the canonical instance itself is happening
| } | ||
|
|
||
| function receivingHost(req: Request) { | ||
| return (req.get("host") || req.hostname || "").toLowerCase(); |
There was a problem hiding this comment.
Let's remove req.hostname -- it's more spoofable and technically has the possibility to differ from req.get("host")
| } | ||
|
|
||
| function isCanonicalPermalinkHost(req: Request) { | ||
| const canonicalHost = new URL(getCommonsCanonicalURL()).hostname.toLowerCase(); |
There was a problem hiding this comment.
We set a CONDUCTOR_DOMAIN env var on each instance (that is the actual domain of the instance, not the primary LibreTexts instance), so we can create a helper like this in server/util/helpers.js:
export const isCanonicalCommonsInstance = () => {
const self = (process.env.CONDUCTOR_DOMAIN || "").trim();
if (!self) return false;
const withProtocol = /^https?:\/\//i.test(self) ? self : `https://${self}`;
try {
return new URL(withProtocol).origin === getCommonsCanonicalURL();
} catch {
return false;
}
};
then we can use
if (!isCanonicalCommonsInstance()) {
// redirect
}
in redirectPermanentLink instead of checking against the request's host
No description provided.