openvibers/openvibe.mediahub Commits

MediaHub review fixes: names resolve through Network's ?username= and only subjects are matched (unknown names refused); downloads carry filename* for any script and a sandbox CSP; a file share's folder route redirects instead of hanging; file names are one line; Media gets an ASCII label

OP OpenVibers committed 8 Oct 2026 · 839a408da6 · 10 files, +59 −35


Changes

ddiff --git a/server/drive/rules.js b/server/drive/rules.js
iindex f477e37..8bbaa2c 100644
---- a/server/drive/rules.js
++++ b/server/drive/rules.js
@@@ -83,7 +83,7 @@ function cleanText(value, max = 500) {
/** A file name as it is stored: one line, no path, never empty. A name is not a path: its slashes become _ . */
function cleanName(value, max = NAME_MAX) {
-- return cleanText(value, max).replace(/[\\/]+/g, '_');
++ return cleanText(value, max).replace(/[\r\n\t]+/g, ' ').replace(/[\\/]+/g, '_').trim();
}
/** The name a file gets: what the person typed, or what the browser said, or a fallback. */
@@@ -92,9 +92,16 @@ function fileName(value, fallback = 'file') {
return s || cleanName(fallback) || 'file';
}
--/** A header-safe rendering of a name for Content-Disposition. */
++/** A header-safe rendering of a name for Content-Disposition: printable ASCII only (the fallback filename=). */
function headerSafeName(name) {
-- return String(name || 'file').replace(/["\\\r\n\t]/g, '_').slice(0, 200) || 'file';
++ return String(name || 'file').replace(/[^\x20-\x7e]/g, '_').replace(/["\\]/g, '_').slice(0, 200) || 'file';
++}
++
++/** attachment; filename="ascii fallback"; filename*=UTF-8''<the real name> — so 日本.pdf downloads as 日本.pdf. */
++function contentDisposition(name) {
++ const real = String(name || 'file').slice(0, 200);
++ const encoded = encodeURIComponent(real).replace(/['()*]/g, (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`);
++ return `attachment; filename="${headerSafeName(real)}"; filename*=UTF-8''${encoded}`;
}
/** An expiry: whole hours, clamped to the product's own range; anything unusable is the default. */
@@@ -131,5 +138,5 @@ module.exports = {
ID_RE, SLUG_RE, isFileId, isFolderId, isUploadId, isSlug, isUsername,
NAME_MAX, FOLDER_NAME_MAX, REASONS, REASON_TEXT, SUSPEND_AT_REPORTS, SUSPEND_IMMEDIATELY,
EXPIRY_MIN_HOURS, EXPIRY_MAX_HOURS, EXPIRY_DEFAULT_HOURS, EXPIRY_CHOICES,
-- normalizeType, safeContentType, cleanName, cleanText, fileName, headerSafeName, expiryHours, formatBytes,
++ normalizeType, safeContentType, cleanName, cleanText, fileName, headerSafeName, contentDisposition, expiryHours, formatBytes,
};
ddiff --git a/server/drive/service.js b/server/drive/service.js
iindex 5fa8c32..32c4221 100644
---- a/server/drive/service.js
++++ b/server/drive/service.js
@@@ -38,17 +38,14 @@ function parseAllow(value) {
/**
* May this viewer open the share? `viewer` is { subject, username } of a signed-in person — the caller has already
-- * refused everyone else. An allow-list is satisfied by the person's canonical subject (resolved when the share was
-- * made) or by their Network username claim (signed, so it needs no directory).
++ * refused everyone else. An allow-list is satisfied only by the person's canonical subject, resolved when the share
++ * was made: a username is a label that can change hands, a subject is the account.
*/
function allowedFor(share, viewer) {
const allow = parseAllow(share.allowed_subjects);
if (!allow) return true;
const subject = viewer && viewer.subject;
-- const username = viewer && viewer.username ? String(viewer.username).toLowerCase() : null;
-- if (subject && allow.subjects.includes(subject)) return true;
-- if (username && allow.usernames.includes(username)) return true;
-- return false;
++ return Boolean(subject && allow.subjects.includes(subject));
}
/** Where a share stands right now: what the page shows and what a download checks. */
@@@ -195,6 +192,9 @@ function createService({ s, media, identity, config, log = console }) {
if (!rules.isUsername(n)) return { ok: false, code: 'share.bad_username', detail: `"${rules.cleanName(n, 40)}" is not an OpenVibe username.` };
}
const resolved = wanted.length ? await identity.resolveUsernames(wanted) : { subjects: [], names: [], unresolved: [] };
++ if (resolved.unresolved.length) {
++ return { ok: false, code: 'share.unknown_user', detail: `No OpenVibe account is called ${resolved.unresolved.map((n) => `@${rules.cleanName(n, 40)}`).join(', ')} (or OpenVibe.Network could not be asked just now). Check the spelling and try again.` };
++ }
const allowed = wanted.length ? { subjects: resolved.subjects, usernames: resolved.names } : null;
const expiresAt = new Date(s.now() + rules.expiryHours(hours) * 3_600_000).toISOString();
const share = await store.insertShare(s, {
ddiff --git a/server/http/api.js b/server/http/api.js
iindex 6e13da0..3c7802b 100644
---- a/server/http/api.js
++++ b/server/http/api.js
@@@ -217,9 +217,6 @@ function createApi(ctx) {
return res.status(201).json({
share: wireShare(out.share),
unresolved: out.unresolved,
-- note: (out.unresolved || []).length
-- ? 'Network could not turn these names into subjects; the share still allows them by their username claim.'
-- : undefined,
});
});
ddiff --git a/server/http/pages.js b/server/http/pages.js
iindex 8cb1de8..f7b42a2 100644
---- a/server/http/pages.js
++++ b/server/http/pages.js
@@@ -408,7 +408,7 @@ ${shares.length ? html`<h3>Its links</h3><ul class="file-list">${shares.map((sh)
crumbs: [{ label: 'Home', href: '/' }, { label: 'Your files', href: '/files' }, { label: row.name, href: `/files/${row.id}` }],
body: html`<h1>Share link made</h1>
<p>The link is <a href="/s/${out.share.slug}"><code>/s/${out.share.slug}</code></a> — it expires ${time(out.share.expires_at)}${out.share.allowed_subjects ? ' and only the people you named can open it' : ''}.</p>
--${out.unresolved && out.unresolved.length ? notice(html`Network could not turn these names into subjects: ${out.unresolved.join(', ')}. The link still allows them by username.`, 'warn') : ''}
++
<p class="muted">Everyone who opens it has to be signed in to OpenVibe before they can download.</p>
<p><a class="sc-btn sc-primary" href="/s/${out.share.slug}">Open the share page</a> <a class="sc-btn" href="/files/${row.id}">Back to the file</a></p>`,
});
@@@ -458,8 +458,10 @@ ${out.unresolved && out.unresolved.length ? notice(html`Network could not turn t
// (HTML, SVG, XML, scripts) is an opaque blob. It is an attachment either way: nothing here is ever
// rendered in a page, on this site or anywhere else.
'Content-Type': rules.safeContentType(row.content_type) || 'application/octet-stream',
-- 'Content-Disposition': `attachment; filename="${rules.headerSafeName(row.name)}"`,
++ 'Content-Disposition': rules.contentDisposition(row.name),
'X-Content-Type-Options': 'nosniff',
++ // Even a browser that ignored the attachment would run nothing from these bytes.
++ 'Content-Security-Policy': "default-src 'none'; sandbox",
'X-Robots-Tag': 'noindex',
'Cache-Control': 'private, no-store',
});
@@@ -582,7 +584,8 @@ ${mine ? '' : reportForm(share)}`,
r.get('/s/:slug/f/:fileId/download', mustSignIn('here', 'A share link still needs an OpenVibe account before anything can be downloaded.'), async (req, res) => {
const share = await openShare(req, res);
-- if (!share || !share.folder_id) return undefined;
++ if (!share) return undefined;
++ if (!share.folder_id) return res.redirect(303, `/s/${share.slug}`); // a file share has no files inside it
const files = await drive.listFiles(share.owner, { folderId: share.folder_id, limit: 1000 });
const file = files.find((f) => f.id === String(req.params.fileId || ''));
if (!file) {
ddiff --git a/server/media/client.js b/server/media/client.js
iindex 159773a..274b54f 100644
---- a/server/media/client.js
++++ b/server/media/client.js
@@@ -22,6 +22,7 @@
* a refusal: it answers { ok: false, status, code, detail } so the caller can show the problem as it came.
*/
const { ids } = require('openvibe-contracts');
++const { headerSafeName } = require('../drive/rules');
/** A user subject as Media wants it in X-OV-Subject: the bare usr_…, or null when it cannot be one. */
function subjectHeader(requester) {
@@@ -70,7 +71,9 @@ function createMediaClient({ config, fetchImpl = globalThis.fetch, log = console
headers: { 'X-OV-Subject': subject },
json: {
kind: 'file', visibility: 'private', size_bytes: size,
-- mime_type: contentType || null, filename: name, multipart: multipart === true,
++ // Media only needs a label: the real name (any script) is MediaHub's, and Media's own download
++ // header takes a filename as is, so it gets the printable-ASCII rendering.
++ mime_type: contentType || null, filename: headerSafeName(name), multipart: multipart === true,
...(multipart && partSize ? { part_size: partSize } : {}),
},
});
ddiff --git a/server/network/identity.js b/server/network/identity.js
iindex 79979dc..d8fa0f0 100644
---- a/server/network/identity.js
++++ b/server/network/identity.js
@@@ -4,17 +4,12 @@
* Resolving the OpenVibe usernames a person names on a share link to canonical subjects, through
* OpenVibe.Network's identity.subject.resolve (network/identity/internal-routes.js):
*
-- * GET {OV_NETWORK_INTERNAL_URL}/internal/identity/resolve?system=&type=&id=
++ * GET {OV_NETWORK_INTERNAL_URL}/internal/identity/resolve?username=<name> (any case; the person's current name)
*
* A service token with audience openvibe.network and scope identity.subject.resolve is required, so it only runs
-- * where the OAuth client is configured. Network keys a subject by its id or by a service-local (system, type, id)
-- * pair; a username is asked for as type=username, and when Network does not know that key the answer is 404 and the
-- * name stays unresolved.
-- *
-- * That is not a dead end: a share's allow-list holds both the subjects we could resolve and the names as typed, and
-- * a download is allowed when the signed-in person's own *subject* is in the resolved list or their Network
-- * *username claim* is in the typed one. The claim is signed by Network, so matching on it needs no directory at
-- * all — the resolve call only sharpens the list (it survives a rename and reaches guests, who have no username).
++ * where the OAuth client is configured. A name Network does not know (404) is unresolved, and a share naming it is
++ * refused: an allow-list holds subjects only. Matching on a typed name would hand the link to whoever registers that
++ * name later, or takes it after a rename.
*/
const { serviceAuth } = require('openvibe-contracts');
@@@ -34,7 +29,7 @@ function createIdentity({ config, fetchImpl = globalThis.fetch, log = console })
if (!tokens) return null;
try {
const res = await fetchImpl(
-- `${config.networkInternalUrl}/internal/identity/resolve?system=network&type=username&id=${encodeURIComponent(username)}`,
++ `${config.networkInternalUrl}/internal/identity/resolve?username=${encodeURIComponent(username)}`,
{ headers: { Accept: 'application/json', ...(await tokens.authHeaders()) }, signal: AbortSignal.timeout(5000) });
if (res.status === 401 && tokens.invalidate) tokens.invalidate();
if (res.status === 404) return null;
@@@ -53,8 +48,8 @@ function createIdentity({ config, fetchImpl = globalThis.fetch, log = console })
/**
* names → { subjects: ['usr_…'], names: ['kim'], unresolved: ['kim'] }.
-- * `names` keeps every typed username (matched against the viewer's claim); `unresolved` is what Network could
-- * not turn into a subject, which the caller reports back to the owner without refusing the share.
++ * `names` keeps every typed username (shown to the owner, never matched); `unresolved` is what Network could not
++ * turn into a subject, and the caller refuses a share that names any.
*/
async function resolveUsernames(list) {
const names = [];
ddiff --git a/test/drive-download.test.js b/test/drive-download.test.js
iindex e2a2d80..40ca48f 100644
---- a/test/drive-download.test.js
++++ b/test/drive-download.test.js
@@@ -70,11 +70,21 @@ const { SAME, bytes, sha256, upload, share } = require('./helpers/drive');
const dl = await t.get(`/files/${file.id}/download`, { as: kim });
assert.strictEqual(dl.status, 200);
const dispo = dl.headers.get('content-disposition');
-- assert.match(dispo, /^attachment; filename="[^"]*"$/, dispo);
++ assert.match(dispo, /^attachment; filename="[^"]*"; filename\*=UTF-8''[A-Za-z0-9%._~!$&+,=@-]*$/, dispo);
assert.ok(!dispo.includes('\n') && !dispo.includes('\r'), dispo);
assert.strictEqual(dl.headers.get('x-injected'), null, 'no header was injected');
});
++ await check('a name in any script downloads under that name, and the bytes run in a sandbox', async () => {
++ const file = await put('日本の写真.png', bytes(30, 'jp'), 'image/png');
++ const dl = await t.get(`/files/${file.id}/download`, { as: kim });
++ assert.strictEqual(dl.status, 200);
++ const dispo = dl.headers.get('content-disposition');
++ assert.ok(dispo.startsWith('attachment; filename="'), dispo);
++ assert.ok(dispo.includes(`filename*=UTF-8''${encodeURIComponent('日本の写真.png')}`), dispo);
++ assert.strictEqual(dl.headers.get('content-security-policy'), "default-src 'none'; sandbox");
++ });
++
await check('a download resumes: a Range is passed through and answered 206', async () => {
const data = bytes(4096, 'range');
const file = await put('slice.bin', data, 'application/octet-stream');
@@@ -83,7 +93,7 @@ const { SAME, bytes, sha256, upload, share } = require('./helpers/drive');
assert.strictEqual(part.buffer.length, 100);
assert.strictEqual(sha256(part.buffer), sha256(data.subarray(100, 200)));
assert.strictEqual(part.headers.get('content-range'), 'bytes 100-199/4096');
-- assert.strictEqual(part.headers.get('content-disposition'), 'attachment; filename="slice.bin"');
++ assert.strictEqual(part.headers.get('content-disposition'), `attachment; filename="slice.bin"; filename*=UTF-8''slice.bin`);
});
await check('nothing is served to anybody who is not signed in', async () => {
ddiff --git a/test/drive-shares.test.js b/test/drive-shares.test.js
iindex 78025f3..429d844 100644
---- a/test/drive-shares.test.js
++++ b/test/drive-shares.test.js
@@@ -65,7 +65,7 @@ const HOUR = 3_600_000;
assert.strictEqual((await t.get(`/api/v1/files/${file.id}`, { as: kim })).json().shares.find((x) => x.slug === slug).downloads, 0);
const dl = await t.get(`/s/${slug}/download`, { as: sam });
assert.strictEqual(dl.status, 200);
-- assert.strictEqual(dl.headers.get('content-disposition'), 'attachment; filename="holiday.bin"');
++ assert.strictEqual(dl.headers.get('content-disposition'), `attachment; filename="holiday.bin"; filename*=UTF-8''holiday.bin`);
assert.strictEqual((await t.get(`/api/v1/files/${file.id}`, { as: kim })).json().shares.find((x) => x.slug === slug).downloads, 1, 'the download is counted');
});
@@@ -97,8 +97,8 @@ const HOUR = 3_600_000;
const r = await share(t, kim, { file_id: file.id, hours: 24, usernames: ['sam'] });
assert.strictEqual(r.status, 201, r.text);
const slug = r.json().share.slug;
-- assert.deepStrictEqual(r.json().share.allowed, { subjects: [], usernames: ['sam'] }, 'Network could not resolve the name, so it is kept as typed');
-- assert.deepStrictEqual(r.json().unresolved, ['sam']);
++ assert.deepStrictEqual(r.json().share.allowed, { subjects: [sam.subject], usernames: ['sam'] }, 'the name is resolved to the account; only the subject is matched');
++ assert.deepStrictEqual(r.json().unresolved, []);
assert.strictEqual((await t.get(`/s/${slug}`, { as: sam })).status, 200, 'sam is named');
assert.strictEqual((await t.get(`/s/${slug}/download`, { as: sam })).status, 200);
const adaPage = await t.get(`/s/${slug}`, { as: ada });
@@@ -108,6 +108,10 @@ const HOUR = 3_600_000;
assert.ok(!adaPage.text.includes('holiday.bin'), 'not even the name');
// The owner is named too, when they name themselves.
assert.strictEqual((await share(t, kim, { file_id: file.id, hours: 24, usernames: ['not a name!'] })).status, 422);
++ // A name no account has is refused: a typed name is never matched later (whoever registers it would get in).
++ const nobody = await share(t, kim, { file_id: file.id, hours: 24, usernames: ['nobody_yet'] });
++ assert.strictEqual(nobody.status, 422, nobody.text);
++ assert.strictEqual(nobody.json().code, 'share.unknown_user');
});
await check('a folder can be shared, one file at a time', async () => {
ddiff --git a/test/helpers/media.js b/test/helpers/media.js
iindex 0d14102..c37eebd 100644
---- a/test/helpers/media.js
++++ b/test/helpers/media.js
@@@ -102,7 +102,7 @@ async function startMedia({ key = KEY, maxSingle = 256 * 1024 * 1024 } = {}) {
const name = String(obj.filename || obj.id).replace(/["\\\r\n]/g, '_');
const headers = {
'Content-Type': obj.mime_type || 'application/octet-stream',
-- 'Content-Disposition': `attachment; filename="${name}"`,
++ 'Content-Disposition': `attachment; filename*=UTF-8''${encodeURIComponent(name)}`, // Media encodes; MediaHub never forwards it
'X-Content-Type-Options': 'nosniff',
'Accept-Ranges': 'bytes',
};
ddiff --git a/test/helpers/mocks.js b/test/helpers/mocks.js
iindex edccd7c..47cacf5 100644
---- a/test/helpers/mocks.js
++++ b/test/helpers/mocks.js
@@@ -260,6 +260,11 @@ async function startNetwork({ sandboxAudiences = ['openvibe.events', 'openvibe.m
// ADR-033: the internal routes a service's account export and deletion answers come back to
// (openvibe-sdk/account-data). Recorded in `.accountParts` and `.accountConfirmations`, as Events' side is
// what a service test needs to see.
++ // identity.subject.resolve by username, as Network answers it (network/identity/internal-routes.js).
++ if (url.pathname === '/internal/identity/resolve' && url.searchParams.get('username')) {
++ const u = st.byUsername.get(String(url.searchParams.get('username')).toLowerCase());
++ return u ? json(200, { subject: { id: u.subject, type: 'user' } }) : problem(json, 404, 'identity.not_found', 'no such username');
++ }
if (/^\/internal\/account-exports\/exp_[0-9A-HJKMNP-TV-Z]{26}\/parts$/.test(url.pathname)) {
st.accountParts.push({ path: url.pathname, body: JSON.parse(raw.toString('utf8') || '{}') });
return json(200, { ok: true });