From 2e23b3b509e8fd900560f5fac2d29d280e795cb5 Mon Sep 17 00:00:00 2001 From: Chad Miller Date: Sat, 15 Aug 2026 15:50:31 -0700 Subject: [PATCH] fix(drive): a private drive is a space of its own type The picker offered every space on the account, so a file could land in a space some other app made. That space is the other app's data model, and its members read what it holds, so the file was both squatting and shared with people the drive never named. It also made the app page attribute that space to pdsjs.dev, since a space shows on an app's page when any collection in it belongs to that app. A private drive is now a space of type dev.pdsjs.drive.space, made from the drive itself, and the picker offers those alone. Every drive space shares one type, so the key names each one and its owner chooses it. Co-Authored-By: Claude Opus 5 (1M context) --- packages/account-ui/src/pages/drive.jsx | 68 +++++++++++- packages/core/src/handlers/account.js | 83 ++++++++++----- packages/drive/src/index.js | 1 + packages/drive/src/lexicon.js | 7 ++ packages/drive/test/account-api.test.js | 136 +++++++++++++++++++----- 5 files changed, 242 insertions(+), 53 deletions(-) diff --git a/packages/account-ui/src/pages/drive.jsx b/packages/account-ui/src/pages/drive.jsx index 17f55e1..04278ed 100644 --- a/packages/account-ui/src/pages/drive.jsx +++ b/packages/account-ui/src/pages/drive.jsx @@ -65,6 +65,63 @@ function Breadcrumb({ ancestry, space, onNavigate }) { ); } +/** + * Make a drive that lives in a permissioned space. Its files are private and + * shared with whoever joins the space, and it is a space of the drive's own + * type: another app's space is that app's data model. + */ +function NewDriveDialog({ onCreate, busy }) { + const [name, setName] = useState(''); + return ( + + New private drive + + } + > +
+

+ A private drive keeps its files out of your public repo. Whoever you + add to it can read them. +

+ setName(event.target.value)} + placeholder="photos" + className="w-full rounded-lg border border-border bg-background px-3 py-2 text-[13px]" + /> +

+ Lowercase letters, numbers and dashes. +

+
+
+ + Cancel + + } + /> + onCreate(name.trim())} + > + Make drive + + } + /> +
+ + ); +} + /** Make a folder here. It is a record of its own, so an empty one stays. */ function NewFolderDialog({ folder, space, onCreate, busy }) { const [name, setName] = useState(''); @@ -230,6 +287,12 @@ export function DrivePage({ data: account, folder, space, onNavigate }) { await post('/drive/folder/delete', { rkey, space: space || undefined }); }); + const makeDrive = (name) => + action.run(async () => { + const made = await post('/drive/space', { name }); + onNavigate(folderHref('', made.uri)); + }); + const empty = data.folders.length === 0 && data.files.length === 0; return ( @@ -250,7 +313,7 @@ export function DrivePage({ data: account, folder, space, onNavigate }) { if (files.length) sendFiles(files); }} > - {data.spaces?.length > 0 && ( + {(data.spaces?.length > 0 || data.canMakeSpace) && (
Drive in ))} + {data.canMakeSpace && ( + + )}
)} diff --git a/packages/core/src/handlers/account.js b/packages/core/src/handlers/account.js index 24ded16..8362960 100644 --- a/packages/core/src/handlers/account.js +++ b/packages/core/src/handlers/account.js @@ -144,30 +144,28 @@ function parseByteRange(header, size) { return { start, end: Math.min(end, size - 1) }; } +// A drive kept in a permissioned space uses a space of its own type. Another +// app's space is that app's data model, and its members read what it holds, so +// a file written there is both squatting and shared with people the drive +// never named. +const DRIVE_SPACE_TYPE = 'dev.pdsjs.drive.space'; + +// A space key is the name its owner gave the drive, so it has to read as one. +const DRIVE_SPACE_KEY = /^[a-z0-9][a-z0-9-]{0,62}$/; + /** - * Name each space for a picker. A space AT-URI ends in its record key, which - * is a TID or the literal `self` and names nothing a person recognises; the - * type NSID is the part that does. The key joins the name only where two - * spaces share a type, since there the type alone would name both. - * @param {Array<{uri: string, spaceType: string}>} rows + * The drives among an account's spaces. Every drive space shares one type, so + * the key is what names each: the owner chose it when the drive was made. + * @param {Array<{uri: string, spaceType: string, deletedAt?: string|null}>} rows * @returns {Array<{uri: string, type: string, key: string, name: string}>} */ -function nameSpaces(rows) { - /** @type {Map} */ - const byType = new Map(); - for (const row of rows) { - byType.set(row.spaceType, (byType.get(row.spaceType) ?? 0) + 1); - } - return rows.map((row) => { - const key = row.uri.split('/').pop() ?? ''; - const short = row.spaceType.split('.').pop() || row.spaceType; - return { - uri: row.uri, - type: row.spaceType, - key, - name: (byType.get(row.spaceType) ?? 0) > 1 ? `${short} (${key})` : short, - }; - }); +function driveSpaces(rows) { + return rows + .filter((row) => row.spaceType === DRIVE_SPACE_TYPE && !row.deletedAt) + .map((row) => { + const key = row.uri.split('/').pop() ?? ''; + return { uri: row.uri, type: row.spaceType, key, name: key }; + }); } /** @@ -2397,10 +2395,13 @@ export function createAccountHandlers(ctx) { ); if (!listing) throw new Error('That folder name is not valid.'); // The spaces this drive can also live in, so the page can offer them. - const rows = spaceBrowser - ? (await spaceBrowser.listSpaces()).filter((space) => !space.deletedAt) - : []; - return { enabled: true, ...listing, spaces: nameSpaces(rows) }; + const rows = spaceBrowser ? await spaceBrowser.listSpaces() : []; + return { + enabled: true, + ...listing, + spaces: driveSpaces(rows), + canMakeSpace: Boolean(spaceAdmin && driveWriter), + }; }); } @@ -2486,6 +2487,34 @@ export function createAccountHandlers(ctx) { ); } + /** + * POST /account/api/drive/space - Make a drive that lives in a permissioned + * space, so its files are private and shared with whoever joins it. + * @param {Request} request @param {URL} url + */ + async function handleApiDriveSpaceCreate(request, url) { + return accountApi( + request, + url, + async (_did, body) => { + if (!driveWriter) throw new Error('Drive is not enabled.'); + if (!spaceAdmin) throw new Error('Spaces are not enabled.'); + const name = typeof body?.name === 'string' ? body.name.trim() : ''; + if (!DRIVE_SPACE_KEY.test(name)) { + throw new Error( + 'Name a drive with lowercase letters, numbers and dashes.', + ); + } + const { uri } = await spaceAdmin.createSpace({ + type: DRIVE_SPACE_TYPE, + skey: name, + }); + return { uri, type: DRIVE_SPACE_TYPE, key: name, name }; + }, + { write: true }, + ); + } + /** * POST /account/api/drive/folder - Make a folder. It is a record of its * own, so a folder with nothing in it stays. @@ -4533,6 +4562,10 @@ export function createAccountHandlers(ctx) { method: 'GET', handler: handleApiDriveChanges, }, + '/account/api/drive/space': { + method: 'POST', + handler: handleApiDriveSpaceCreate, + }, '/account/api/drive/folder': { method: 'POST', handler: handleApiDriveFolderCreate, diff --git a/packages/drive/src/index.js b/packages/drive/src/index.js index 0058928..b53de2d 100644 --- a/packages/drive/src/index.js +++ b/packages/drive/src/index.js @@ -4,6 +4,7 @@ export { createDriveBrowser } from './browser.js'; export { DRIVE_FILE_COLLECTION, DRIVE_FOLDER_COLLECTION, + DRIVE_SPACE_TYPE, driveFileLexicon, driveFolderLexicon, folderAncestry, diff --git a/packages/drive/src/lexicon.js b/packages/drive/src/lexicon.js index 26b43ff..94525aa 100644 --- a/packages/drive/src/lexicon.js +++ b/packages/drive/src/lexicon.js @@ -14,6 +14,13 @@ export const DRIVE_FILE_COLLECTION = 'dev.pdsjs.drive.file'; export const DRIVE_FOLDER_COLLECTION = 'dev.pdsjs.drive.folder'; +/** + * The space type a private drive uses. A drive kept in a permissioned space + * makes one of these rather than borrowing a space some other app made: that + * space is the other app's data model, and its members read what it holds. + */ +export const DRIVE_SPACE_TYPE = 'dev.pdsjs.drive.space'; + /** Longest file or folder name, in characters. */ export const MAX_NAME_LENGTH = 512; diff --git a/packages/drive/test/account-api.test.js b/packages/drive/test/account-api.test.js index f59c038..287d642 100644 --- a/packages/drive/test/account-api.test.js +++ b/packages/drive/test/account-api.test.js @@ -52,7 +52,7 @@ async function fileRecord(file) { } /** - * @param {{drive?: boolean, readOnly?: boolean, upload?: boolean, spaces?: Array<{uri: string, spaceType: string}>}} [opts] + * @param {{drive?: boolean, readOnly?: boolean, upload?: boolean, spaces?: Array<{uri: string, spaceType: string, deletedAt?: string}>}} [opts] */ async function makePds(opts = {}) { /** @type {Map} */ @@ -86,6 +86,7 @@ async function makePds(opts = {}) { const deleted = /** @type {string[]} */ ([]); const unlinked = /** @type {string[]} */ ([]); + const created = /** @type {Array<{type: string, skey: string}>} */ ([]); // The repo event log, which the change feed reads to build a delta. /** @type {Array<{seq: number, evt: Uint8Array}>} */ const events = []; @@ -179,10 +180,20 @@ async function makePds(opts = {}) { spaceBrowser: opts.spaces ? /** @type {any} */ ({ listSpaces: async () => - opts.spaces?.map((space) => ({ ...space, deletedAt: null })), + opts.spaces?.map((space) => ({ deletedAt: null, ...space })), countRecords: async () => [], }) : undefined, + spaceAdmin: opts.spaces + ? /** @type {any} */ ({ + createSpace: async ( + /** @type {{type: string, skey: string}} */ made, + ) => { + created.push(made); + return { uri: `at://${DID}/space/${made.type}/${made.skey}` }; + }, + }) + : undefined, }); // Commit and reap run their full course in integration; here the API // contract is under test, not the repo machinery. @@ -209,7 +220,15 @@ async function makePds(opts = {}) { return []; } ); - return { pds, deleted, unlinked, commits, records, reapCount: () => reaped }; + return { + pds, + deleted, + unlinked, + commits, + records, + created, + reapCount: () => reaped, + }; } /** @param {PersonalDataServer} pds */ @@ -747,68 +766,131 @@ describe('the change feed as a client would walk it', () => { }); }); -describe('naming the spaces a drive can live in', () => { +describe('which spaces a drive is offered', () => { const uriFor = (/** @type {string} */ type, /** @type {string} */ key) => `at://${DID}/space/${type}/${key}`; - it('names a space by its type, not by its record key', async () => { - // A key is a TID or the literal `self`, which names nothing a person - // recognises. The type NSID is the part that does. + it("offers only spaces made for a drive, never another app's", async () => { const { pds } = await makePds({ spaces: [ + // An app's own space holds that app's data model, and its members + // read what is in it. A file put there is squatting and is shared. + { + uri: uriFor('app.bulleted.space', 'self'), + spaceType: 'app.bulleted.space', + }, { uri: uriFor('sh.tangled.repo', '3msw4lgrmisi4'), spaceType: 'sh.tangled.repo', }, { - uri: uriFor('com.example.notes', 'self'), - spaceType: 'com.example.notes', + uri: uriFor('dev.pdsjs.drive.space', 'photos'), + spaceType: 'dev.pdsjs.drive.space', }, ], }); const cookie = await signInCookie(pds); const { body } = await api(pds, '/account/api/drive/files', { cookie }); - expect(body.spaces.map((/** @type {any} */ s) => s.name)).toEqual([ - 'repo', - 'notes', + expect(body.spaces).toEqual([ + { + uri: uriFor('dev.pdsjs.drive.space', 'photos'), + type: 'dev.pdsjs.drive.space', + key: 'photos', + name: 'photos', + }, ]); }); - it('joins the key only where two spaces share a type', async () => { + it('names a drive by the key its owner chose', async () => { const { pds } = await makePds({ spaces: [ - { uri: uriFor('sh.tangled.repo', 'aaa'), spaceType: 'sh.tangled.repo' }, - { uri: uriFor('sh.tangled.repo', 'bbb'), spaceType: 'sh.tangled.repo' }, { - uri: uriFor('com.example.notes', 'self'), - spaceType: 'com.example.notes', + uri: uriFor('dev.pdsjs.drive.space', 'photos'), + spaceType: 'dev.pdsjs.drive.space', + }, + { + uri: uriFor('dev.pdsjs.drive.space', 'work'), + spaceType: 'dev.pdsjs.drive.space', }, ], }); const cookie = await signInCookie(pds); const { body } = await api(pds, '/account/api/drive/files', { cookie }); + // Every drive space shares one type, so the key is what tells them apart. expect(body.spaces.map((/** @type {any} */ s) => s.name)).toEqual([ - 'repo (aaa)', - 'repo (bbb)', - 'notes', + 'photos', + 'work', ]); }); - it('carries the type and key beside the name', async () => { + it('leaves out a deleted drive', async () => { const { pds } = await makePds({ spaces: [ { - uri: uriFor('sh.tangled.repo', 'self'), - spaceType: 'sh.tangled.repo', + uri: uriFor('dev.pdsjs.drive.space', 'gone'), + spaceType: 'dev.pdsjs.drive.space', + deletedAt: '2026-08-15T00:00:00.000Z', }, ], }); const cookie = await signInCookie(pds); const { body } = await api(pds, '/account/api/drive/files', { cookie }); - expect(body.spaces[0]).toMatchObject({ - type: 'sh.tangled.repo', - key: 'self', - name: 'repo', + expect(body.spaces).toEqual([]); + }); +}); + +describe('/account/api/drive/space', () => { + it('makes a space of the drive type, not of any other', async () => { + const { pds, created } = await makePds({ spaces: [] }); + const cookie = await signInCookie(pds); + const { status, body } = await api(pds, '/account/api/drive/space', { + cookie, + method: 'POST', + body: { name: 'photos' }, }); + expect(status).toBe(200); + expect(created).toEqual([ + { type: 'dev.pdsjs.drive.space', skey: 'photos' }, + ]); + expect(body).toMatchObject({ key: 'photos', name: 'photos' }); + }); + + it('refuses a name that would not read as one', async () => { + const { pds } = await makePds({ spaces: [] }); + const cookie = await signInCookie(pds); + for (const name of ['', 'Photos', 'my photos', 'a/b', '-lead']) { + const { status } = await api(pds, '/account/api/drive/space', { + cookie, + method: 'POST', + body: { name }, + }); + expect(status).toBe(400); + } + }); + + it('reports the feature off where spaces are not enabled', async () => { + const { pds } = await makePds(); + const cookie = await signInCookie(pds); + const { status, body } = await api(pds, '/account/api/drive/space', { + cookie, + method: 'POST', + body: { name: 'photos' }, + }); + expect(status).toBe(400); + expect(body.error).toMatch(/not enabled/); + + const listing = await api(pds, '/account/api/drive/files', { cookie }); + expect(listing.body.canMakeSpace).toBe(false); + }); + + it('is blocked in read-only mode', async () => { + const { pds } = await makePds({ spaces: [], readOnly: true }); + const cookie = await signInCookie(pds); + const { status } = await api(pds, '/account/api/drive/space', { + cookie, + method: 'POST', + body: { name: 'photos' }, + }); + expect(status).toBe(403); }); }); -- 2.51.2