feat: avatar by link, loaded by the browser; Files nests workspaces under owners' homes
- POST /api/me/avatar/url stores the https link (users.avatar_link) instead of downloading it; GET /api/me/avatar and /api/users/:id/avatar answer 302 to it (no referrer). Link rules: https, no userinfo, <= 2048 chars, no localhost or private/reserved IP literal. Upload and link replace each other. - Settings -> Account first loads the link as an embedded image and refuses one this browser cannot show: a picture behind a login (Mattermost's /api/v4/users/<id>/image) gets no cookie cross-site and 401s. - Files sidebar: each workspace under its owner's home, chevrons remembered per browser (agenthub.files.homesOpen); ownerless ones stay flat.
This commit is contained in:
@@ -7,8 +7,9 @@
|
||||
* another account never sees it through `/api/me`; staff read it at
|
||||
* `/api/users/:id/avatar` (a developer is refused) and a workspace carries
|
||||
* `owner_avatar_at` for the sidebar's owner groups. A picture from a link
|
||||
* (`POST /api/me/avatar/url`, server/lib/avatar-url.js) takes https only and
|
||||
* never reaches a loopback/private address.
|
||||
* (`POST /api/me/avatar/url`, server/lib/avatar-url.js) is stored, never
|
||||
* downloaded, and the avatar routes redirect to it; https only, never a
|
||||
* loopback/private IP literal.
|
||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||
*/
|
||||
|
||||
@@ -110,6 +111,46 @@ it("refuses an SVG, an empty body, a too-large image and no login", async () =>
|
||||
assert.equal((await put("", PNG)).status, 401);
|
||||
});
|
||||
|
||||
it("a picture from a link is stored and redirected to, never downloaded", async () => {
|
||||
const ava = await login("ava", "ava-pass-1");
|
||||
const ann = await login("ann", "ann-pass-1");
|
||||
const link =
|
||||
"https://chat.runsystem.vn/api/v4/users/7bya4wy58bdk3qmb5ks1rgoqkh/image?_=1776312011651";
|
||||
assert.equal((await put(ava, PNG)).status, 200);
|
||||
const set = await fetch(BASE + "/api/me/avatar/url", {
|
||||
method: "POST",
|
||||
headers: { cookie: ava, "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ url: ` ${link} ` }),
|
||||
});
|
||||
assert.equal(set.status, 200);
|
||||
assert.ok((await set.json()).user.avatarAt);
|
||||
// The uploaded file is gone: a link replaces it.
|
||||
assert.equal(fs.readdirSync(path.join(TMP, "avatars")).length, 0);
|
||||
|
||||
const got = await fetch(BASE + "/api/me/avatar", {
|
||||
headers: { cookie: ava },
|
||||
redirect: "manual",
|
||||
});
|
||||
assert.equal(got.status, 302);
|
||||
assert.equal(got.headers.get("location"), link);
|
||||
assert.equal(got.headers.get("referrer-policy"), "no-referrer");
|
||||
const id = auth.findUser("ava").id;
|
||||
const staff = await fetch(`${BASE}/api/users/${id}/avatar`, {
|
||||
headers: { cookie: ann },
|
||||
redirect: "manual",
|
||||
});
|
||||
assert.equal(staff.status, 302);
|
||||
assert.equal(staff.headers.get("location"), link);
|
||||
|
||||
// An upload replaces the link.
|
||||
assert.equal((await put(ava, PNG)).status, 200);
|
||||
assert.equal(
|
||||
(await fetch(BASE + "/api/me/avatar", { headers: { cookie: ava }, redirect: "manual" })).status,
|
||||
200
|
||||
);
|
||||
await fetch(BASE + "/api/me/avatar", { method: "DELETE", headers: { cookie: ava } });
|
||||
});
|
||||
|
||||
it("a picture from a link: https only, never a private or local address", async () => {
|
||||
const ava = await login("ava", "ava-pass-1");
|
||||
const post = (url) =>
|
||||
|
||||
+4
-3
@@ -1933,9 +1933,10 @@ try {
|
||||
}
|
||||
// An account's own display name and email (the Users page's Edit user and
|
||||
// Settings → Account), and when its avatar was last set (server/lib/avatars.js
|
||||
// — the image is a file, the column only versions its URL). Additive; after
|
||||
// the rebuild above, which predates them.
|
||||
for (const col of ["display_name", "email", "avatar_at"]) {
|
||||
// — the image is a file, the column only versions its URL — or `avatar_link`,
|
||||
// a link the avatar routes redirect to). Additive; after the rebuild above,
|
||||
// which predates them.
|
||||
for (const col of ["display_name", "email", "avatar_at", "avatar_link"]) {
|
||||
try {
|
||||
db.prepare(`SELECT ${col} FROM users LIMIT 1`).get();
|
||||
} catch {
|
||||
|
||||
@@ -0,0 +1,21 @@
|
||||
/**
|
||||
* @file avatar-redirect.js
|
||||
* @description Answers an avatar GET (`/api/me/avatar`, `/api/users/:id/avatar`)
|
||||
* with a 302 to the account's stored avatar LINK (server/lib/avatars.js), so
|
||||
* the viewer's browser loads the picture itself — with its own logins and
|
||||
* network — and the server never fetches it. No referrer is sent with it.
|
||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||
*/
|
||||
|
||||
/** True when the account has a link and the redirect was sent. */
|
||||
function sendAvatarLink(res, userId) {
|
||||
const link = require("./avatars").avatarLink(userId);
|
||||
if (!link) return false;
|
||||
res.set("Referrer-Policy", "no-referrer");
|
||||
// The client versions the URL with `avatarAt`, so a new link is a new URL.
|
||||
res.set("Cache-Control", "private, max-age=86400");
|
||||
res.redirect(302, link);
|
||||
return true;
|
||||
}
|
||||
|
||||
module.exports = { sendAvatarLink };
|
||||
+22
-100
@@ -1,26 +1,19 @@
|
||||
/**
|
||||
* @file avatar-url.js
|
||||
* @description Fetches an avatar picture from a URL the account typed
|
||||
* (Settings → Account, `POST /api/me/avatar/url`), so the server — not the
|
||||
* browser, which most image hosts' CORS would block — downloads it. A URL the
|
||||
* user names makes the SERVER connect somewhere, so it is held to: `https://`
|
||||
* with no userinfo, a host whose EVERY resolved address is public (no
|
||||
* loopback, private, link-local, CGNAT, multicast or reserved range — the
|
||||
* dashboard's own network is never reachable this way), the connection pinned
|
||||
* to the vetted address (a second DNS answer cannot rebind it), at most
|
||||
* `MAX_REDIRECTS` redirects each re-checked the same way, a timeout, and at
|
||||
* most `MAX_AVATAR_BYTES` read. No cookie or credential is ever sent. The
|
||||
* bytes then go through `setAvatar`'s own magic-byte check like an upload.
|
||||
* @description Checks an avatar LINK the account typed (Settings → Account,
|
||||
* `POST /api/me/avatar/url`). The link is stored as it is and the avatar
|
||||
* routes redirect the viewer's browser to it — the server never downloads it,
|
||||
* so a picture behind the viewer's own login or on a company-internal host
|
||||
* (e.g. a Mattermost `/api/v4/users/<id>/image`) still shows. Held to:
|
||||
* `https://`, no userinfo, at most `MAX_LINK_LENGTH` characters, and no
|
||||
* loopback/private/link-local/reserved IP literal as the host (a link may not
|
||||
* aim every viewer's browser at their own machine or LAN).
|
||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||
*/
|
||||
|
||||
const dns = require("node:dns");
|
||||
const https = require("node:https");
|
||||
const net = require("node:net");
|
||||
const { MAX_AVATAR_BYTES } = require("./avatars");
|
||||
|
||||
const MAX_REDIRECTS = 3;
|
||||
const TIMEOUT_MS = 10_000;
|
||||
const MAX_LINK_LENGTH = 2048;
|
||||
|
||||
const fail = (code, message) => Object.assign(new Error(message), { code });
|
||||
|
||||
@@ -75,100 +68,29 @@ function isPublicAddress(address) {
|
||||
return !blocked.check(address, "ipv6");
|
||||
}
|
||||
|
||||
/** The URL as a `URL`, or EBADURL: https only, no userinfo, a real host. */
|
||||
/** The link, normalised, or EBADURL / EPRIVATEURL: https only, no userinfo, a real host. */
|
||||
function parseAvatarUrl(text) {
|
||||
const raw = String(text ?? "").trim();
|
||||
if (raw.length > MAX_LINK_LENGTH) throw fail("EBADURL", "the link is too long");
|
||||
let url;
|
||||
try {
|
||||
url = new URL(String(text ?? "").trim());
|
||||
url = new URL(raw);
|
||||
} catch {
|
||||
throw fail("EBADURL", "not a valid URL");
|
||||
}
|
||||
if (url.protocol !== "https:") throw fail("EBADURL", "only an https:// link");
|
||||
if (url.username || url.password) throw fail("EBADURL", "a link with a login is refused");
|
||||
if (!url.hostname) throw fail("EBADURL", "not a valid URL");
|
||||
// Node never calls `lookup` for an IP-literal host, so it is checked here.
|
||||
const literal = url.hostname.replace(/^\[|\]$/g, "");
|
||||
if (net.isIP(literal) && !isPublicAddress(literal)) {
|
||||
const host = url.hostname.toLowerCase();
|
||||
const literal = host.replace(/^\[|\]$/g, "");
|
||||
if (
|
||||
host === "localhost" ||
|
||||
host.endsWith(".localhost") ||
|
||||
(net.isIP(literal) && !isPublicAddress(literal))
|
||||
) {
|
||||
throw fail("EPRIVATEURL", "that address is not on the public internet");
|
||||
}
|
||||
return url;
|
||||
return url.href;
|
||||
}
|
||||
|
||||
/**
|
||||
* `lookup` for `https.request`: resolves every address and refuses the host
|
||||
* when ANY of them is not public, then hands over only vetted ones.
|
||||
*/
|
||||
function safeLookup(hostname, options, cb) {
|
||||
if (typeof options === "function") [cb, options] = [options, {}];
|
||||
const literal = hostname.replace(/^\[|\]$/g, "");
|
||||
const done = (addrs) => {
|
||||
if (!addrs.length || !addrs.every((a) => isPublicAddress(a.address))) {
|
||||
return cb(fail("EPRIVATEURL", "that address is not on the public internet"));
|
||||
}
|
||||
if (options?.all) return cb(null, addrs);
|
||||
return cb(null, addrs[0].address, addrs[0].family);
|
||||
};
|
||||
if (net.isIP(literal)) return done([{ address: literal, family: net.isIP(literal) }]);
|
||||
dns.lookup(hostname, { all: true, verbatim: true }, (err, addrs) => {
|
||||
if (err) return cb(fail("EFETCH", `could not resolve ${hostname}`));
|
||||
done(addrs);
|
||||
});
|
||||
}
|
||||
|
||||
function getOnce(url) {
|
||||
return new Promise((resolve, reject) => {
|
||||
const req = https.get(
|
||||
url,
|
||||
{
|
||||
lookup: safeLookup,
|
||||
agent: false,
|
||||
timeout: TIMEOUT_MS,
|
||||
headers: { Accept: "image/png,image/jpeg,image/gif,image/webp", "User-Agent": "AgentHub" },
|
||||
},
|
||||
(res) => {
|
||||
const status = res.statusCode || 0;
|
||||
if (status >= 300 && status < 400 && res.headers.location) {
|
||||
res.resume();
|
||||
return resolve({ redirect: new URL(res.headers.location, url).href });
|
||||
}
|
||||
if (status !== 200) {
|
||||
res.resume();
|
||||
return reject(fail("EFETCH", `the server answered ${status}`));
|
||||
}
|
||||
if (Number(res.headers["content-length"]) > MAX_AVATAR_BYTES) {
|
||||
res.destroy();
|
||||
return reject(fail("ETOOBIG", "the image is too large"));
|
||||
}
|
||||
const chunks = [];
|
||||
let size = 0;
|
||||
res.on("data", (chunk) => {
|
||||
size += chunk.length;
|
||||
if (size > MAX_AVATAR_BYTES) {
|
||||
res.destroy();
|
||||
return reject(fail("ETOOBIG", "the image is too large"));
|
||||
}
|
||||
chunks.push(chunk);
|
||||
});
|
||||
res.on("end", () => resolve({ body: Buffer.concat(chunks) }));
|
||||
res.on("error", (e) => reject(e.code ? e : fail("EFETCH", e.message)));
|
||||
}
|
||||
);
|
||||
req.on("timeout", () => req.destroy(fail("EFETCH", "the server took too long")));
|
||||
req.on("error", (e) =>
|
||||
reject(e.code === "EPRIVATEURL" || e.code === "EFETCH" ? e : fail("EFETCH", e.message))
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
/** Downloads the picture at `text`; resolves to its bytes (unchecked — `setAvatar` sniffs them). */
|
||||
async function fetchAvatarImage(text) {
|
||||
let url = parseAvatarUrl(text);
|
||||
for (let hop = 0; hop <= MAX_REDIRECTS; hop++) {
|
||||
const out = await getOnce(url);
|
||||
if (out.body) return out.body;
|
||||
url = parseAvatarUrl(out.redirect);
|
||||
}
|
||||
throw fail("EFETCH", "too many redirects");
|
||||
}
|
||||
|
||||
module.exports = { fetchAvatarImage, isPublicAddress, parseAvatarUrl };
|
||||
module.exports = { MAX_LINK_LENGTH, isPublicAddress, parseAvatarUrl };
|
||||
|
||||
+43
-6
@@ -5,7 +5,9 @@
|
||||
* `<data dir>/avatars/u<id>.<png|jpg|gif|webp>` — a name made from the
|
||||
* account id and the type its magic bytes say, never from the request — and
|
||||
* `users.avatar_at` records when it was set, so the client can version the
|
||||
* URL. Raster only: an SVG served from the dashboard's origin runs script.
|
||||
* URL. Or, instead of a file, a LINK (`users.avatar_link`, checked by
|
||||
* avatar-url.js) the serving routes redirect the browser to — never
|
||||
* downloaded. Setting one clears the other. Raster only: an SVG served from the dashboard's origin runs script.
|
||||
* At most `MAX_AVATAR_BYTES`; the serving route adds `nosniff` and a
|
||||
* `sandbox` CSP like every other user-supplied file.
|
||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||
@@ -32,10 +34,35 @@ function fileName(userId, ext) {
|
||||
|
||||
const db = () => require("../db").db;
|
||||
|
||||
/** Deletes every stored avatar file of the account and clears `avatar_at`. */
|
||||
function removeAvatar(userId) {
|
||||
const removeFiles = (userId) => {
|
||||
for (const ext of EXTS) fs.rmSync(fileName(userId, ext), { force: true });
|
||||
db().prepare("UPDATE users SET avatar_at = NULL WHERE id = ?").run(Number(userId));
|
||||
};
|
||||
|
||||
/** Deletes every stored avatar file of the account and clears `avatar_at` and the link. */
|
||||
function removeAvatar(userId) {
|
||||
removeFiles(userId);
|
||||
db()
|
||||
.prepare("UPDATE users SET avatar_at = NULL, avatar_link = NULL WHERE id = ?")
|
||||
.run(Number(userId));
|
||||
}
|
||||
|
||||
/** Stores `text` (EBADURL / EPRIVATEURL) as the account's avatar link; returns `avatar_at`. */
|
||||
function setAvatarLink(userId, text) {
|
||||
const link = require("./avatar-url").parseAvatarUrl(text);
|
||||
removeFiles(userId);
|
||||
const at = new Date().toISOString();
|
||||
db()
|
||||
.prepare("UPDATE users SET avatar_at = ?, avatar_link = ? WHERE id = ?")
|
||||
.run(at, link, Number(userId));
|
||||
return at;
|
||||
}
|
||||
|
||||
/** The account's avatar link, or null when it has none. */
|
||||
function avatarLink(userId) {
|
||||
return (
|
||||
db().prepare("SELECT avatar_link FROM users WHERE id = ?").get(Number(userId))?.avatar_link ||
|
||||
null
|
||||
);
|
||||
}
|
||||
|
||||
/** Stores `buf` as the account's avatar (EBADIMAGE / ETOOBIG); returns `avatar_at`. */
|
||||
@@ -54,7 +81,9 @@ function setAvatar(userId, buf) {
|
||||
if (other !== ext) fs.rmSync(fileName(userId, other), { force: true });
|
||||
}
|
||||
const at = new Date().toISOString();
|
||||
db().prepare("UPDATE users SET avatar_at = ? WHERE id = ?").run(at, Number(userId));
|
||||
db()
|
||||
.prepare("UPDATE users SET avatar_at = ?, avatar_link = NULL WHERE id = ?")
|
||||
.run(at, Number(userId));
|
||||
return at;
|
||||
}
|
||||
|
||||
@@ -68,4 +97,12 @@ function avatarFile(userId) {
|
||||
return null;
|
||||
}
|
||||
|
||||
module.exports = { MAX_AVATAR_BYTES, setAvatar, removeAvatar, avatarFile, avatarsDir };
|
||||
module.exports = {
|
||||
MAX_AVATAR_BYTES,
|
||||
setAvatar,
|
||||
setAvatarLink,
|
||||
removeAvatar,
|
||||
avatarFile,
|
||||
avatarLink,
|
||||
avatarsDir,
|
||||
};
|
||||
|
||||
@@ -14,7 +14,7 @@
|
||||
* - GET/PATCH /api/me/account, POST /api/me/password, POST /api/me/sign-out-others
|
||||
* (Auth) — Settings → Account: the account's own profile, password and logins
|
||||
* - GET/PUT/DELETE /api/me/avatar (Auth) — the account's own picture
|
||||
* - POST /api/me/avatar/url (Auth) — the same picture downloaded from a link
|
||||
* - POST /api/me/avatar/url (Auth) — the picture as a link, redirected to
|
||||
* - GET/PATCH /api/users/{id} (Auth) — the Users page's Edit user form
|
||||
* - GET /api/users/{id}/avatar (Auth) — one account's picture (admin/analyst)
|
||||
* - GET /api/users/tmux, POST /api/users/{id}/tmux-restart, POST /api/users/tmux-restart-idle
|
||||
@@ -309,9 +309,10 @@ const paths = {
|
||||
get: {
|
||||
tags: ["Auth"],
|
||||
operationId: "getMyAvatar",
|
||||
summary: "The account's own picture (nosniff, sandbox CSP)",
|
||||
summary: "The account's own picture (nosniff, sandbox CSP), or a 302 to its avatar link",
|
||||
responses: {
|
||||
200: { description: "The image (PNG, JPEG, GIF or WebP)" },
|
||||
302: { description: "Redirect to the account's avatar link" },
|
||||
401: err("EAUTH"),
|
||||
404: err("ENOAVATAR"),
|
||||
},
|
||||
@@ -500,7 +501,7 @@ const paths = {
|
||||
tags: ["Auth"],
|
||||
operationId: "setMyAvatarFromUrl",
|
||||
summary:
|
||||
"Set the account's picture from an https link the server downloads (public addresses only, at most 3 redirects, PNG/JPEG/GIF/WebP, at most 1 MB). Login cookie only, audited",
|
||||
"Set the account's picture to an https link, stored and never downloaded — the avatar GETs answer 302 to it (no userinfo, no localhost/private IP literal, at most 2048 chars). Login cookie only, audited",
|
||||
requestBody: {
|
||||
required: true,
|
||||
content: {
|
||||
@@ -515,11 +516,9 @@ const paths = {
|
||||
},
|
||||
responses: {
|
||||
200: { description: "Set", content: accountBody },
|
||||
400: err("EBADURL / EPRIVATEURL / EBADIMAGE"),
|
||||
400: err("EBADURL / EPRIVATEURL"),
|
||||
401: err("EAUTH — no login cookie (a pane token is refused)"),
|
||||
403: err("ELOCALMODE"),
|
||||
413: err("ETOOBIG"),
|
||||
502: err("EFETCH — the link could not be downloaded"),
|
||||
},
|
||||
},
|
||||
},
|
||||
@@ -531,6 +530,7 @@ const paths = {
|
||||
parameters: [{ name: "id", in: "path", required: true, schema: { type: "integer" } }],
|
||||
responses: {
|
||||
200: { description: "The image (PNG, JPEG, GIF or WebP)" },
|
||||
302: { description: "Redirect to that account's avatar link" },
|
||||
403: err("EADMIN — a developer"),
|
||||
404: err("ENOAVATAR"),
|
||||
},
|
||||
|
||||
+7
-5
@@ -27,8 +27,8 @@
|
||||
* owner) may not change the owner's login. Each change is audited.
|
||||
* `/avatar` (server mode) is the account's own picture (server/lib/avatars.js):
|
||||
* GET serves it (`nosniff`, `sandbox` CSP), PUT takes the raw image bytes
|
||||
* (PNG/JPEG/GIF/WebP, at most 1 MB), `POST /avatar/url` downloads the same
|
||||
* from an https link (public addresses only, server/lib/avatar-url.js) and
|
||||
* (PNG/JPEG/GIF/WebP, at most 1 MB), `POST /avatar/url` stores an https link
|
||||
* instead, which `GET /avatar` redirects to (server/lib/avatar-url.js) and
|
||||
* DELETE removes it — all writes are account writes like the profile's. The sidebar shows it beside the name.
|
||||
*
|
||||
* S3 (one Linux user per account): an account's dark/light choice also becomes
|
||||
@@ -264,11 +264,13 @@ router.post(
|
||||
);
|
||||
|
||||
const avatars = require("../lib/avatars");
|
||||
const { sendAvatarLink } = require("../lib/avatar-redirect");
|
||||
|
||||
router.get("/avatar", (req, res) => {
|
||||
if (!auth.isServerMode()) return res.status(404).json({ error: { code: "ENOAVATAR" } });
|
||||
const user = req.user || auth.userFromRequest(req);
|
||||
if (!user) return unauth(res);
|
||||
if (sendAvatarLink(res, user.id)) return;
|
||||
const file = avatars.avatarFile(user.id);
|
||||
if (!file) return res.status(404).json({ error: { code: "ENOAVATAR", message: "no avatar" } });
|
||||
res.set("Content-Type", file.type);
|
||||
@@ -289,13 +291,13 @@ router.put(
|
||||
})
|
||||
);
|
||||
|
||||
// The same picture from a link: the server downloads it (server/lib/avatar-url.js).
|
||||
// A picture by link: stored as it is, never downloaded — GET /avatar redirects
|
||||
// the browser to it (server/lib/avatar-url.js).
|
||||
router.post(
|
||||
"/avatar/url",
|
||||
sameOriginGuard,
|
||||
accountWrite(async (_req, user, body) => {
|
||||
const buf = await require("../lib/avatar-url").fetchAvatarImage(body.url);
|
||||
avatars.setAvatar(user.id, buf);
|
||||
avatars.setAvatarLink(user.id, body.url);
|
||||
return { out: accountView(user.id), action: "own avatar set from a link" };
|
||||
})
|
||||
);
|
||||
|
||||
@@ -209,6 +209,12 @@ router.get("/:id", (req, res) => {
|
||||
|
||||
router.get("/:id/avatar", (req, res) => {
|
||||
const id = Number(req.params.id);
|
||||
if (
|
||||
Number.isSafeInteger(id) &&
|
||||
id > 0 &&
|
||||
require("../lib/avatar-redirect").sendAvatarLink(res, id)
|
||||
)
|
||||
return;
|
||||
const file = Number.isSafeInteger(id) && id > 0 ? require("../lib/avatars").avatarFile(id) : null;
|
||||
if (!file) return res.status(404).json({ error: { code: "ENOAVATAR", message: "no avatar" } });
|
||||
res.set("Content-Type", file.type);
|
||||
|
||||
Reference in New Issue
Block a user