mirror of
https://github.com/webadderallorg/Recordly.git
synced 2026-09-28 16:55:36 +00:00
fix(cloud): scope owner access and protect shared response data
This commit is contained in:
@@ -49,4 +49,4 @@ Restart `npm run dev` after creating `.env.local`. Open an editor and verify:
|
||||
3. Google opens the system browser and returns to Recordly.
|
||||
4. Clicking **Create link** while signed out opens this modal; after successful authentication it continues to the share dialog.
|
||||
|
||||
Add the same `SUPABASE_URL` and `SUPABASE_PUBLISHABLE_KEY` values to the share Worker's secrets or variables. The Worker validates the user's access token with Supabase before accepting an upload. `API_SECRET` is server-side only and remains available for library administration and explicitly enabled local integration tests; it is never entered into or exposed by the desktop app.
|
||||
Add the same `SUPABASE_URL` and `SUPABASE_PUBLISHABLE_KEY` values to the share Worker's secrets or variables. Set `OWNER_USER_ID` to the owner’s Supabase user ID. The Worker validates the access token with Supabase and requires that owner identity before accepting API requests. `API_SECRET` is server-side only and remains available for library administration and explicitly enabled local integration tests; it is never entered into or exposed by the desktop app.
|
||||
|
||||
@@ -8,7 +8,7 @@ The desktop app defaults to the local development endpoint:
|
||||
http://localhost:8787/api/upload
|
||||
```
|
||||
|
||||
The endpoint is intentionally not user-configurable. Development builds use the local service above; production builds use `https://videos.recordly.dev/api/upload`. Publishing requires the user's Recordly access token. No share API secret is exposed in the app.
|
||||
The endpoint is intentionally not user-configurable. All builds currently use the local service above. Production service integration is planned but is not selected by any build. Publishing requires the user's Recordly access token. No share API secret is exposed in the app.
|
||||
|
||||
## Publishing protocol
|
||||
|
||||
@@ -26,3 +26,5 @@ Anyone with a valid link can watch a public recording and leave timestamped feed
|
||||
## Third-party licensing
|
||||
|
||||
The hosting service is based on MIT-licensed open-source software. Required attribution and the complete license text are preserved in [THIRD_PARTY_NOTICES.md](../THIRD_PARTY_NOTICES.md) and [the vendored license](../services/recordly-share/LICENSE).
|
||||
|
||||
The self-hosted worker is a single-owner library. Set `OWNER_USER_ID` to the owner’s Supabase user ID. Other accounts in the same Supabase project cannot administer the library. Missing owner configuration disables Supabase bearer access.
|
||||
|
||||
@@ -12,3 +12,6 @@ ALLOW_API_SECRET_UPLOADS=false
|
||||
# signed-in users can publish without seeing an API-secret field.
|
||||
SUPABASE_URL=https://YOUR_PROJECT_REF.supabase.co
|
||||
SUPABASE_PUBLISHABLE_KEY=sb_publishable_YOUR_KEY
|
||||
|
||||
# Only this Supabase user may administer this single-owner deployment.
|
||||
OWNER_USER_ID=
|
||||
|
||||
@@ -9,3 +9,6 @@ API_SECRET=
|
||||
SUPABASE_URL=
|
||||
SUPABASE_PUBLISHABLE_KEY=
|
||||
ALLOW_API_SECRET_UPLOADS=false
|
||||
|
||||
# Only this Supabase user may administer this single-owner deployment.
|
||||
OWNER_USER_ID=
|
||||
|
||||
@@ -12,7 +12,7 @@ npm --prefix web run build
|
||||
npm run dev
|
||||
```
|
||||
|
||||
Set `SUPABASE_URL` and `SUPABASE_PUBLISHABLE_KEY` in `.dev.vars` to the same public project configuration used by the desktop app. Recordly sends the signed-in user's access token when it publishes to:
|
||||
Set `OWNER_USER_ID` to the deployment owner’s Supabase user ID. Only that user may administer this single-owner library. Set `SUPABASE_URL` and `SUPABASE_PUBLISHABLE_KEY` in `.dev.vars` to the same public project configuration used by the desktop app. Recordly sends the signed-in user's access token when it publishes to:
|
||||
|
||||
- Endpoint: `http://localhost:8787/api/upload`
|
||||
|
||||
@@ -31,7 +31,7 @@ npx wrangler secret put SUPABASE_PUBLISHABLE_KEY
|
||||
npm run deploy
|
||||
```
|
||||
|
||||
Set `SUPABASE_URL` as a Worker variable. The ID-less configuration provisions `recordly-share-db` and `recordly-videos` for a new deployment. Add the `videos.recordly.dev` custom domain; production Recordly builds accept only that publishing origin.
|
||||
Set `SUPABASE_URL` as a Worker variable. The ID-less configuration provisions `recordly-share-db` and `recordly-videos` for a new deployment. Desktop builds currently upload only to localhost; production service integration is deferred.
|
||||
|
||||
## Attribution
|
||||
|
||||
|
||||
@@ -375,7 +375,7 @@ async function isAuthorized(request, env) {
|
||||
env.API_SECRET &&
|
||||
timingSafeEqual(token, env.API_SECRET)
|
||||
) return true;
|
||||
if (!env.SUPABASE_URL || !env.SUPABASE_PUBLISHABLE_KEY) return false;
|
||||
if (!env.SUPABASE_URL || !env.SUPABASE_PUBLISHABLE_KEY || !env.OWNER_USER_ID) return false;
|
||||
try {
|
||||
const authBase = new URL(env.SUPABASE_URL);
|
||||
const isLocal = authBase.hostname === 'localhost' || authBase.hostname === '127.0.0.1';
|
||||
@@ -387,7 +387,9 @@ async function isAuthorized(request, env) {
|
||||
apikey: env.SUPABASE_PUBLISHABLE_KEY,
|
||||
},
|
||||
});
|
||||
return response.ok;
|
||||
if (!response.ok) return false;
|
||||
const user = await response.json();
|
||||
return typeof user.id === 'string' && timingSafeEqual(user.id, env.OWNER_USER_ID);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
@@ -448,7 +450,10 @@ async function expectedSessionToken(env) {
|
||||
}
|
||||
|
||||
async function isDashboardAuthed(request, env) {
|
||||
if (await isAuthorized(request, env)) return true;
|
||||
return (await isAuthorized(request, env)) || dashboardCookieAuthed(request, env);
|
||||
}
|
||||
|
||||
async function dashboardCookieAuthed(request, env) {
|
||||
const cookies = parseCookies(request.headers.get('Cookie') || '');
|
||||
const sessionToken = cookies['voom_session'];
|
||||
if (!sessionToken) return false;
|
||||
@@ -611,8 +616,7 @@ async function handleRequest(request, env) {
|
||||
});
|
||||
}
|
||||
|
||||
const cookieOk = await isDashboardAuthed(request, env);
|
||||
if (!(await isAuthorized(request, env)) && !cookieOk) {
|
||||
if (!(await isAuthorized(request, env)) && !(await dashboardCookieAuthed(request, env))) {
|
||||
return errorResponse('Unauthorized', 401);
|
||||
}
|
||||
|
||||
@@ -807,6 +811,11 @@ async function handleRequest(request, env) {
|
||||
|
||||
// --- API Handlers ---
|
||||
|
||||
function finiteNonnegative(value) {
|
||||
const number = Number(value);
|
||||
return Number.isFinite(number) && number >= 0 ? number : 0;
|
||||
}
|
||||
|
||||
async function handleUpload(request, env) {
|
||||
const body = await request.json();
|
||||
const { title, duration, width, height, hasWebcam, fileSize, password_hash, cta_url, cta_text } = body;
|
||||
@@ -836,7 +845,7 @@ async function handleUpload(request, env) {
|
||||
`INSERT INTO videos (share_code, title, duration, width, height, has_webcam, file_size, expires_at, password_hash, password_salt, cta_url, cta_text)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`
|
||||
)
|
||||
.bind(shareCode, title, duration || 0, width || 0, height || 0, hasWebcam ? 1 : 0, fileSize || 0, expiresAt, storedHash, salt, cta_url || null, cta_text || null)
|
||||
.bind(shareCode, title, finiteNonnegative(duration), finiteNonnegative(width), finiteNonnegative(height), hasWebcam ? 1 : 0, finiteNonnegative(fileSize), expiresAt, storedHash, salt, cta_url || null, cta_text || null)
|
||||
.run();
|
||||
|
||||
const baseUrl = new URL(request.url).origin;
|
||||
@@ -1086,7 +1095,7 @@ async function handleVideoStream(request, env, shareCode) {
|
||||
'Content-Range': `bytes ${start}-${actualEnd}/${totalSize}`,
|
||||
'Content-Length': String(actualEnd - start + 1),
|
||||
'Accept-Ranges': 'bytes',
|
||||
'Cache-Control': 'public, max-age=3600',
|
||||
'Cache-Control': video.password_hash ? 'private, no-store' : 'public, max-age=3600',
|
||||
'Access-Control-Allow-Origin': '*',
|
||||
'Access-Control-Expose-Headers': 'Content-Range, Content-Length, Accept-Ranges',
|
||||
},
|
||||
@@ -1103,7 +1112,7 @@ async function handleVideoStream(request, env, shareCode) {
|
||||
'Content-Type': 'video/mp4',
|
||||
'Content-Length': String(object.size),
|
||||
'Accept-Ranges': 'bytes',
|
||||
'Cache-Control': 'public, max-age=3600',
|
||||
'Cache-Control': video.password_hash ? 'private, no-store' : 'public, max-age=3600',
|
||||
'Access-Control-Allow-Origin': '*',
|
||||
'Access-Control-Expose-Headers': 'Content-Range, Content-Length, Accept-Ranges',
|
||||
},
|
||||
@@ -1140,7 +1149,7 @@ async function handleVTT(request, env, shareCode) {
|
||||
return new Response(vtt, {
|
||||
headers: {
|
||||
'Content-Type': 'text/vtt; charset=utf-8',
|
||||
'Cache-Control': 'public, max-age=3600',
|
||||
'Cache-Control': video.password_hash ? 'private, no-store' : 'public, max-age=3600',
|
||||
'Access-Control-Allow-Origin': '*',
|
||||
},
|
||||
});
|
||||
@@ -1249,8 +1258,8 @@ async function handleOGPage(request, env, shareCode) {
|
||||
<meta property="og:video" content="${baseUrl}/embed/${shareCode}">
|
||||
<meta property="og:video:secure_url" content="${baseUrl}/embed/${shareCode}">
|
||||
<meta property="og:video:type" content="text/html">
|
||||
<meta property="og:video:width" content="${video.width}">
|
||||
<meta property="og:video:height" content="${video.height}">
|
||||
<meta property="og:video:width" content="${finiteNonnegative(video.width)}">
|
||||
<meta property="og:video:height" content="${finiteNonnegative(video.height)}">
|
||||
<meta property="og:image" content="${baseUrl}/og/${shareCode}">
|
||||
<meta property="og:image:secure_url" content="${baseUrl}/og/${shareCode}">
|
||||
<meta property="og:image:width" content="1200">
|
||||
@@ -1262,8 +1271,8 @@ async function handleOGPage(request, env, shareCode) {
|
||||
<meta name="twitter:description" content="${desc}">
|
||||
<meta name="twitter:image" content="${baseUrl}/og/${shareCode}">
|
||||
<meta name="twitter:player" content="${baseUrl}/embed/${shareCode}">
|
||||
<meta name="twitter:player:width" content="${video.width}">
|
||||
<meta name="twitter:player:height" content="${video.height}">
|
||||
<meta name="twitter:player:width" content="${finiteNonnegative(video.width)}">
|
||||
<meta name="twitter:player:height" content="${finiteNonnegative(video.height)}">
|
||||
</head>
|
||||
<body>
|
||||
<p>${escapeHTML(video.title)}</p>
|
||||
@@ -1459,8 +1468,10 @@ async function handleGetComments(request, env, shareCode) {
|
||||
if (!authed) return errorResponse('Password required', 401);
|
||||
|
||||
const url = new URL(request.url);
|
||||
const page = Math.max(1, parseInt(url.searchParams.get('page') || '1', 10));
|
||||
const limit = Math.min(parseInt(url.searchParams.get('limit') || '50', 10), 100);
|
||||
const requestedPage = parseInt(url.searchParams.get('page') || '1', 10);
|
||||
const requestedLimit = parseInt(url.searchParams.get('limit') || '50', 10);
|
||||
const page = Number.isSafeInteger(requestedPage) ? Math.max(1, Math.min(requestedPage, Math.floor(Number.MAX_SAFE_INTEGER / 100))) : 1;
|
||||
const limit = Number.isFinite(requestedLimit) ? Math.max(1, Math.min(requestedLimit, 100)) : 50;
|
||||
const offset = (page - 1) * limit;
|
||||
|
||||
const total = await env.DB.prepare(
|
||||
@@ -1545,7 +1556,7 @@ async function handleOGImage(env, shareCode) {
|
||||
const date = formatDate(video.created_at);
|
||||
const rawTitle = locked ? 'Protected video' : video.title;
|
||||
const title = rawTitle.length > 60 ? rawTitle.substring(0, 57) + '...' : rawTitle;
|
||||
const res = !locked && video.width > 0 ? `${video.width}\u00d7${video.height}` : '';
|
||||
const res = !locked && video.width > 0 ? `${finiteNonnegative(video.width)}\u00d7${finiteNonnegative(video.height)}` : '';
|
||||
|
||||
const svg = `<svg xmlns="http://www.w3.org/2000/svg" width="1200" height="630" viewBox="0 0 1200 630">
|
||||
<rect width="1200" height="630" fill="#000"/>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, it, expect, beforeAll } from 'vitest';
|
||||
import { describe, it, expect, beforeAll, vi } from 'vitest';
|
||||
import { env, SELF } from 'cloudflare:test';
|
||||
import { sha256Hex } from '../src/index.js';
|
||||
import worker, { sha256Hex } from '../src/index.js';
|
||||
|
||||
const AUTH = { Authorization: 'Bearer test-secret' };
|
||||
const BASE = 'https://share.test';
|
||||
@@ -448,3 +448,59 @@ describe('error middleware', () => {
|
||||
expect((await res.json()).error).toBe('Internal error');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe('review security fixes', () => {
|
||||
it('allows only the configured Supabase owner and performs one auth lookup', async () => {
|
||||
const config = { ...env, ALLOW_API_SECRET_UPLOADS: 'false', SUPABASE_URL: 'https://auth.example.test', SUPABASE_PUBLISHABLE_KEY: 'public-test', OWNER_USER_ID: 'owner' };
|
||||
const request = () => new Request(`${BASE}/api/health`, { headers: { Authorization: 'Bearer user-token' } });
|
||||
const lookup = vi.spyOn(globalThis, 'fetch');
|
||||
try {
|
||||
lookup.mockResolvedValue(new Response(JSON.stringify({ id: 'other-user' })));
|
||||
expect((await worker.fetch(request(), config, {})).status).toBe(401);
|
||||
lookup.mockClear().mockResolvedValue(new Response(JSON.stringify({ id: 'owner' })));
|
||||
expect((await worker.fetch(request(), config, {})).status).toBe(200);
|
||||
expect(lookup).toHaveBeenCalledTimes(1);
|
||||
lookup.mockClear();
|
||||
expect((await worker.fetch(request(), { ...config, OWNER_USER_ID: '' }, {})).status).toBe(401);
|
||||
expect(lookup).not.toHaveBeenCalled();
|
||||
} finally { lookup.mockRestore(); }
|
||||
});
|
||||
|
||||
it('never publicly caches protected video or transcripts', async () => {
|
||||
const password = 'cache-test';
|
||||
const { shareCode } = await createShare({ password_hash: await sha256Hex(password) });
|
||||
await completeUpload(shareCode);
|
||||
const unlocked = await SELF.fetch(`${BASE}/s/${shareCode}/verify-password`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ password }) });
|
||||
const Cookie = unlocked.headers.get('Set-Cookie').split(';')[0];
|
||||
for (const [url, extra] of [[`/v/${shareCode}`, {}], [`/v/${shareCode}`, { Range: 'bytes=0-3' }], [`/vtt/${shareCode}`, {}]]) {
|
||||
const response = await SELF.fetch(`${BASE}${url}`, { headers: { Cookie, ...extra } });
|
||||
expect([200, 206]).toContain(response.status);
|
||||
expect(response.headers.get('Cache-Control')).toBe('private, no-store');
|
||||
await response.arrayBuffer();
|
||||
}
|
||||
});
|
||||
|
||||
it('coerces untrusted dimensions at upload and when rendering older rows', async () => {
|
||||
const payload = '\"><script>alert(1)</script>';
|
||||
const { shareCode } = await createShare({ width: payload, height: payload, duration: 'invalid', fileSize: -1 });
|
||||
const row = await env.DB.prepare('SELECT width, height, duration, file_size FROM videos WHERE share_code = ?').bind(shareCode).first();
|
||||
expect(row).toMatchObject({ width: 0, height: 0, duration: 0, file_size: 0 });
|
||||
await completeUpload(shareCode);
|
||||
await env.DB.prepare('UPDATE videos SET width = ?, height = ? WHERE share_code = ?').bind(payload, payload, shareCode).run();
|
||||
const response = await SELF.fetch(`${BASE}/s/${shareCode}`, { headers: { 'User-Agent': 'Twitterbot/1.0' } });
|
||||
expect(await response.text()).not.toContain('<script>alert(1)</script>');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
it('normalizes invalid comment pagination and clamps zero limits', async () => {
|
||||
const { shareCode } = await createShare();
|
||||
await completeUpload(shareCode);
|
||||
for (const query of ['page=invalid&limit=invalid', 'page=-5&limit=0', 'page=1&limit=-1']) {
|
||||
const response = await SELF.fetch(`${BASE}/s/${shareCode}/comments?${query}`);
|
||||
expect(response.status).toBe(200);
|
||||
const body = await response.json();
|
||||
expect(body.comments).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -4,10 +4,8 @@
|
||||
// database and R2 bucket in the DEPLOYER's account. The worker creates its own
|
||||
// schema at runtime, so no migrations need to run.
|
||||
//
|
||||
// wrangler picks JSON config over wrangler.toml, so a bare `wrangler deploy`
|
||||
// here uses THIS file. The maintainer's own worker (with real resource IDs)
|
||||
// lives in wrangler.toml — always deploy it with `npm run deploy`
|
||||
// (== `wrangler deploy --config wrangler.toml`), never a bare `wrangler deploy`.
|
||||
// npm run deploy uses this wrangler.jsonc configuration.
|
||||
// Supply a separate explicit --config path for any deployment-specific configuration.
|
||||
//
|
||||
"name": "recordly-share",
|
||||
"main": "src/index.js",
|
||||
|
||||
Reference in New Issue
Block a user