From 8a70fa2e5cd443e0cb2d3f9c3d5e48067596ecfc Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Tue, 8 Sep 2026 22:56:03 -0700 Subject: [PATCH] fix: CODEOWNERS + perm dialogs outside puter dashboard/desktop (#3833) --- .github/CODEOWNERS | 65 ++++++++++++++++- src/gui/src/UI/UIPermissionDialog.js | 51 ++++++++++++-- src/gui/src/UI/UIPermissionDialog.test.js | 85 ++++++++++++++++++++++- 3 files changed, 191 insertions(+), 10 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 245a5f7df..91e635e7c 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,3 +1,64 @@ -* @Salazareo @ProgrammerIn-wonderland @jelveh @jfcastro92 +# Default owner, including backend and SDK areas without a specialist below. +# The last matching rule takes precedence. +* @Salazareo -/src/docs/ @Salazareo @ProgrammerIn-wonderland @jelveh @jfcastro92 @reynaldichernando +# GUI +/src/gui/ @jelveh @Salazareo + +# Documentation, developer site, and CLI +/doc/ @reynaldichernando +/src/docs/ @reynaldichernando +/src/dev-center/ @reynaldichernando +/src/cli/ @reynaldichernando + +# Backend: AI, hosting, workers, networking, and WebDAV +/src/backend/controllers/puterai/ @ProgrammerIn-wonderland +/src/backend/controllers/hosting/ @ProgrammerIn-wonderland +/src/backend/controllers/webdav/ @ProgrammerIn-wonderland +/src/backend/controllers/wisp/ @ProgrammerIn-wonderland +/src/backend/controllers/peer/ @ProgrammerIn-wonderland +/src/backend/drivers/ai-*/ @ProgrammerIn-wonderland +/src/backend/drivers/util/ @ProgrammerIn-wonderland +/src/backend/drivers/subdomain/ @ProgrammerIn-wonderland +/src/backend/drivers/workers/ @ProgrammerIn-wonderland +/src/backend/services/subdomain/ @ProgrammerIn-wonderland +/src/backend/services/localworker/ @ProgrammerIn-wonderland +/src/backend/services/socket/ @ProgrammerIn-wonderland +/src/backend/stores/subdomain/ @ProgrammerIn-wonderland +/extensions/workerSandbox* @ProgrammerIn-wonderland +/src/worker/ @ProgrammerIn-wonderland +/src/worker-types/ @ProgrammerIn-wonderland + +# Backend: sharing, permissions, and teams +/src/backend/controllers/share/ @jfcastro92 +/src/backend/controllers/team/ @jfcastro92 +/src/backend/services/share/ @jfcastro92 +/src/backend/services/permission/ @jfcastro92 +/src/backend/services/acl/ @jfcastro92 +/src/backend/services/team/ @jfcastro92 +/src/backend/services/apps/AppPermissionService* @jfcastro92 +/src/backend/stores/share/ @jfcastro92 +/src/backend/stores/permission/ @jfcastro92 +/src/backend/stores/group/ @jfcastro92 +/src/backend/stores/team/ @jfcastro92 + +# SDK: AI, hosting, workers, and networking +/src/puter-js/src/modules/ai/ @ProgrammerIn-wonderland +/src/puter-js/src/modules/hosting/ @ProgrammerIn-wonderland +/src/puter-js/src/modules/Workers* @ProgrammerIn-wonderland +/src/puter-js/src/modules/networking/ @ProgrammerIn-wonderland +/src/puter-js/src/modules/Peer* @ProgrammerIn-wonderland +/src/puter-js/src/lib/networkUtils* @ProgrammerIn-wonderland +/src/puter-js/tests/api/suites/ai.suite.ts @ProgrammerIn-wonderland +/src/puter-js/tests/api/suites/hosting.suite.ts @ProgrammerIn-wonderland +/src/puter-js/tests/api/suites/workers.suite.ts @ProgrammerIn-wonderland +/src/puter-js/tests/api/suites/net.suite.ts @ProgrammerIn-wonderland + +# SDK: sharing and permissions +/src/puter-js/src/modules/perms/ @jfcastro92 +/src/puter-js/src/modules/FileSystem/operations/share* @jfcastro92 +/src/puter-js/src/modules/FileSystem/operations/unshare* @jfcastro92 +/src/puter-js/src/modules/FileSystem/operations/getShares* @jfcastro92 +/src/puter-js/src/modules/FileSystem/operations/listShared* @jfcastro92 +/src/puter-js/tests/api/suites/perms.suite.ts @jfcastro92 +/src/puter-js/tests/api/suites/sharing.suite.ts @jfcastro92 diff --git a/src/gui/src/UI/UIPermissionDialog.js b/src/gui/src/UI/UIPermissionDialog.js index ea44ca137..35aae78e3 100644 --- a/src/gui/src/UI/UIPermissionDialog.js +++ b/src/gui/src/UI/UIPermissionDialog.js @@ -734,6 +734,36 @@ async function get_app_by_uid (uid) { } } +/** + * The uid of the app making the request, for the describers that compare it + * against a uid the permission itself names. + * + * The in-GUI path knows the uid outright (IPC.js passes it); the popup and + * iframe flows know the requester only by origin, so it is resolved here. It is + * resolved for the description alone and never sent to /auth/grant-user-app, + * which keeps receiving the origin and re-resolving it server-side: an origin + * with no app row of its own resolves to a synthetic `app-`, + * and the grant endpoint reads its `app_uid` as uid-or-name, so a forwarded + * synthetic uid would hand the grant to whoever registered an app under that + * literal name. + * + * Null when there is nothing to resolve or the lookup fails — + * `getAppUIDFromOrigin` reports failure by resolving to a null or undefined uid + * rather than throwing (a refused origin comes back as an error body with no + * uid), and throws only when the call itself cannot be made. + */ +async function resolve_requesting_app_uid (options) { + if ( options.app_uid ) return options.app_uid; + if ( ! options.origin ) return null; + try { + const uid = await window.getAppUIDFromOrigin(options.origin); + return uid ?? null; + } catch (e) { + console.error('Failed to resolve requesting app', options.origin, e); + return null; + } +} + /** * Describes `app-data:[:[:]]` — one app using another's data. * @@ -747,7 +777,12 @@ export async function get_app_data_description (parts, options) { if ( ! target_uid ) return null; // An app already reaches its own data; approving that would mean nothing. - if ( options.app_uid && target_uid === options.app_uid ) return null; + // A requester that will not resolve is not fatal here, unlike in the + // delegation describer below: this check only suppresses a prompt that + // would mean nothing, and denying on a failed lookup would refuse a + // cross-app request that is perfectly good. + const requester_app_uid = await resolve_requesting_app_uid(options); + if ( requester_app_uid && target_uid === requester_app_uid ) return null; const app = await get_app_by_uid(target_uid); if ( ! app ) return null; @@ -791,17 +826,21 @@ export async function get_app_data_description (parts, options) { * region of the data it keeps for this user to other people it picks. * * Returns null (which denies without prompting) for anything this copy cannot - * honestly bound: another user's data, a namespace that is not the requester's - * own, or a request naming no region — that last one is the whole of the app's - * data, which is a different decision and not one a prompt can put in a line. + * honestly bound: another user's data, a requester this copy cannot name, a + * namespace that is not the requester's own, or a request naming no region — + * that last one is the whole of the app's data, which is a different decision + * and not one a prompt can put in a line. */ export async function get_kv_share_description (parts, options) { const [, , owner_uuid, namespace_app_uid, ...segments] = parts; if ( ! owner_uuid || ! namespace_app_uid || segments.length === 0 ) return null; // An app reaches its own namespace and no other, so a request naming - // another one describes access it could not use. - if ( ! options.app_uid || namespace_app_uid !== options.app_uid ) return null; + // another one describes access it could not use. Checking that at all + // needs the requester named, so an origin that will not resolve is refused + // here rather than prompted for. + const requester_app_uid = await resolve_requesting_app_uid(options); + if ( ! requester_app_uid || namespace_app_uid !== requester_app_uid ) return null; const whoami = await puter.auth.whoami(); if ( whoami.uuid !== owner_uuid ) return null; diff --git a/src/gui/src/UI/UIPermissionDialog.test.js b/src/gui/src/UI/UIPermissionDialog.test.js index ef0b77f7e..f560897ce 100644 --- a/src/gui/src/UI/UIPermissionDialog.test.js +++ b/src/gui/src/UI/UIPermissionDialog.test.js @@ -16,6 +16,7 @@ const { get_app_data_description, get_kv_share_description } = const CONTACTS = 'app-contacts'; const CALENDAR = 'app-calendar'; const OWNER = '2a1b0c9d-0000-4000-8000-000000000001'; +const SITE = 'https://calendar.example'; /** Stub the app lookup the describer performs. */ const stubApp = (app) => { @@ -25,12 +26,18 @@ const stubApp = (app) => { })); }; +/** Stub the origin → app-uid lookup the popup flow's describers perform. */ +const stubOriginApp = (uid) => { + window.getAppUIDFromOrigin = vi.fn(async () => uid); +}; + const describeScope = (permission, options = { app_uid: CALENDAR }) => get_app_data_description(permission.split(':'), options); describe('UIPermissionDialog app-data descriptions', () => { beforeEach(() => { stubApp({ uid: CONTACTS, name: 'contacts', title: 'Contacts' }); + delete window.getAppUIDFromOrigin; }); it('names the target app and the read verb', async () => { @@ -106,12 +113,29 @@ describe('UIPermissionDialog app-data descriptions', () => { }); expect(await describeScope(`app-data:${CONTACTS}:kv:get`)).toBeNull(); }); + + it('refuses a site’s request for its own data', async () => { + // The popup flow reaches the same self-request check the in-GUI flow does. + stubOriginApp(CONTACTS); + expect( + await describeScope(`app-data:${CONTACTS}:kv:get`, { origin: SITE }), + ).toBeNull(); + }); + + it('still describes a cross-app request when the requester will not resolve', async () => { + // An unresolvable requester only costs the self-request check here; it + // must not deny a valid cross-app request. + stubOriginApp(null); + const d = await describeScope(`app-data:${CONTACTS}:kv:get`, { origin: SITE }); + expect(d.html).toContain('perm_app_data_read'); + }); }); describe('UIPermissionDialog key-value delegation descriptions', () => { beforeEach(() => { stubApp({ uid: CALENDAR, name: 'calendar', title: 'Calendar' }); globalThis.puter = { auth: { whoami: async () => ({ uuid: OWNER }) } }; + delete window.getAppUIDFromOrigin; }); const describeShare = (permission, options = { app_uid: CALENDAR }) => @@ -142,15 +166,72 @@ describe('UIPermissionDialog key-value delegation descriptions', () => { `manage:kv-share:${OWNER}:${CONTACTS}:workspace:abc`, ), ).toBeNull(); - // No requesting app at all — the popup flow — cannot be bounded either. + }); + + it('describes a delegation from a site that resolves to the namespace’s app', async () => { + // The popup flow: the requester arrives as an origin, not a uid. + stubOriginApp(CALENDAR); + const d = await describeShare( + `manage:kv-share:${OWNER}:${CALENDAR}:workspace:abc`, + { origin: SITE }, + ); + expect(d.html).toContain('perm_kv_share_manage'); + expect(d.html).toContain('Calendar'); + expect(d.html).toContain('region=workspace:abc:'); + expect(window.getAppUIDFromOrigin).toHaveBeenCalledWith(SITE); + }); + + it('refuses a site that resolves to a different app', async () => { + stubOriginApp(CONTACTS); expect( await describeShare( `manage:kv-share:${OWNER}:${CALENDAR}:workspace:abc`, - { origin: 'https://site.example' }, + { origin: SITE }, ), ).toBeNull(); }); + it('refuses a site whose app uid does not resolve', async () => { + stubOriginApp(null); + expect( + await describeShare( + `manage:kv-share:${OWNER}:${CALENDAR}:workspace:abc`, + { origin: SITE }, + ), + ).toBeNull(); + // The lookup reports failure by value, both shapes. + window.getAppUIDFromOrigin = vi.fn(async () => undefined); + expect( + await describeShare( + `manage:kv-share:${OWNER}:${CALENDAR}:workspace:abc`, + { origin: SITE }, + ), + ).toBeNull(); + }); + + it('refuses when the lookup itself throws', async () => { + // A failed lookup must deny, never prompt. + window.getAppUIDFromOrigin = vi.fn(async () => { + throw new Error('network down'); + }); + expect( + await describeShare( + `manage:kv-share:${OWNER}:${CALENDAR}:workspace:abc`, + { origin: SITE }, + ), + ).toBeNull(); + }); + + it('keeps naming the requester by uid when the GUI supplies one', async () => { + // A stub that would deny if it were consulted. + stubOriginApp(CONTACTS); + const d = await describeShare( + `manage:kv-share:${OWNER}:${CALENDAR}:workspace:abc`, + ); + expect(d.html).toContain('perm_kv_share_manage'); + expect(window.getAppUIDFromOrigin).not.toHaveBeenCalled(); + }); + it('refuses another user’s data', async () => { expect( await describeShare(