From 7fdc8715fec2ca06733119510b579f005ed78adc Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 22 May 2026 14:22:25 -0400 Subject: [PATCH 1/2] fix(cloud-tests): unblock GCP picker when folder enumeration is forbidden MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Customer Propper hit a real failure that PR #2899's folder enumeration could not handle. Verified in production CloudWatch logs: WARN [GCPSecurityService] Failed to list child folders of organizations/43356919874: { "error": { "code": 403, "message": "The caller does not have permission", "status": "PERMISSION_DENIED" } } LOG [GCPSecurityService] GCP detectProjectsForOrg(43356919874): 13 direct + 0 folder-nested → 13 unique Two coordinated changes: 1. Switch `cloudresourcemanager.googleapis.com/v2/folders` → `cloudresourcemanager.googleapis.com/v3/folders`. v2 is deprecated and was observed returning 403 PERMISSION_DENIED for OAuth grants that legitimately had org-level folder roles (roles/owner + roles/resourcemanager.folderAdmin). v3 is the current API and accepts the same `parent`/`pageSize`/`pageToken` query params, so the swap is purely defensive — response shape is identical. 2. Add a broad-query fallback in `listProjectsInOrgFolderTree`. When folder enumeration returns zero folder IDs (whether due to 403, 404, no folders existing, or anything else), retry with a broader `lifecycleState:ACTIVE AND parent.type:folder` query. That returns every folder-nested project the OAuth user can `projects.get`, which solves Greg's case without depending on the folders endpoint succeeding. Multi-org tradeoff: in a multi-org tenant the fallback path may include folder-nested projects from other orgs the user has access to. This is acceptable because: - the picker is selection-based — the user chooses what to monitor, - the alternative is a silently empty picker like Greg saw, - the user already authorized those projects via their IAM grant. When v3/folders works as expected, the precise per-folder query is preferred and the fallback never fires — verified by a new test that locks in this behavior. Tests: - 2 new regression tests on `gcp-security.service.spec.ts`: - customer-Propper scenario: v3/folders empty → broad fallback fires → folder-nested projects appear. - healthy multi-org scenario: v3/folders succeeds → precise scoping preserved, broad fallback does not fire. - All 15 GCP detection tests pass; full cloud-security suite (269 tests) passes; one pre-existing TLS env failure is unrelated. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../providers/gcp-security.service.spec.ts | 135 +++++++++++++++--- .../providers/gcp-security.service.ts | 55 ++++++- 2 files changed, 167 insertions(+), 23 deletions(-) diff --git a/apps/api/src/cloud-security/providers/gcp-security.service.spec.ts b/apps/api/src/cloud-security/providers/gcp-security.service.spec.ts index fc8627ec4..93df054bd 100644 --- a/apps/api/src/cloud-security/providers/gcp-security.service.spec.ts +++ b/apps/api/src/cloud-security/providers/gcp-security.service.spec.ts @@ -170,7 +170,8 @@ describe('GCPSecurityService — project detection', () => { // ─── detectProjectsForOrg: org direct + folder-tree projects ────────── /** - * Build a Response-like for the v2/folders endpoint. + * Build a Response-like for the v3/folders endpoint (response shape + * is identical to the legacy v2/folders endpoint). */ function foldersPage(opts: { folders: string[]; // folder IDs (numeric) @@ -203,11 +204,11 @@ describe('GCPSecurityService — project detection', () => { const seenFolderIdsQueried: string[] = []; fetchMock.mockImplementation(async (url: string) => { // Folder enumeration: top-level folders under the org. - if (url.includes('v2/folders') && url.includes('organizations%2F43356919874')) { + if (url.includes('v3/folders') && url.includes('organizations%2F43356919874')) { return foldersPage({ folders: ['9724350536'] }); } // Folder enumeration: sub-folders (none in this tree). - if (url.includes('v2/folders') && url.includes('folders%2F9724350536')) { + if (url.includes('v3/folders') && url.includes('folders%2F9724350536')) { return foldersPage({ folders: [] }); } // Direct org children projects. @@ -250,13 +251,13 @@ describe('GCPSecurityService — project detection', () => { // └── folder 3000 (nested) // └── project deep-prod fetchMock.mockImplementation(async (url: string) => { - if (url.includes('v2/folders') && url.includes('organizations%2F1000')) { + if (url.includes('v3/folders') && url.includes('organizations%2F1000')) { return foldersPage({ folders: ['2000'] }); } - if (url.includes('v2/folders') && url.includes('folders%2F2000')) { + if (url.includes('v3/folders') && url.includes('folders%2F2000')) { return foldersPage({ folders: ['3000'] }); } - if (url.includes('v2/folders') && url.includes('folders%2F3000')) { + if (url.includes('v3/folders') && url.includes('folders%2F3000')) { return foldersPage({ folders: [] }); } if (url.includes('v1/projects') && url.includes('parent.id%3A1000')) { @@ -281,10 +282,10 @@ describe('GCPSecurityService — project detection', () => { it('dedupes when the same project would appear in both arms', async () => { fetchMock.mockImplementation(async (url: string) => { - if (url.includes('v2/folders') && url.includes('organizations%2F123')) { + if (url.includes('v3/folders') && url.includes('organizations%2F123')) { return foldersPage({ folders: ['folder-a'] }); } - if (url.includes('v2/folders') && url.includes('folders%2Ffolder-a')) { + if (url.includes('v3/folders') && url.includes('folders%2Ffolder-a')) { return foldersPage({ folders: [] }); } if (url.includes('v1/projects') && url.includes('parent.id%3A123')) { @@ -302,9 +303,10 @@ describe('GCPSecurityService — project detection', () => { expect(result.map((p) => p.id)).toEqual(['proj-shared', 'proj-unique']); }); - it('returns empty array when the org has no direct projects and no folders', async () => { + it('returns empty array when both arms (and the broad-query fallback) come back empty', async () => { fetchMock.mockImplementation(async (url: string) => { - if (url.includes('v2/folders')) return foldersPage({ folders: [] }); + if (url.includes('v3/folders')) return foldersPage({ folders: [] }); + // Catches both the direct arm AND the broad-query fallback. if (url.includes('v1/projects')) return gcpPage({ projects: [] }); throw new Error(`Unexpected URL: ${url}`); }); @@ -313,13 +315,108 @@ describe('GCPSecurityService — project detection', () => { expect(result).toEqual([]); }); + it('falls back to broad parent.type:folder query when folder enumeration returns empty (customer Propper regression)', async () => { + // Verified failure mode in production: v3/folders (and v2 before + // it) returns `403 PERMISSION_DENIED` for some OAuth grants + // despite the user having org-level folder roles. Folder + // enumeration silently returns []. The picker would have been + // missing folder-nested projects. + // + // After the fallback: a broad `parent.type:folder` query (no + // parent.id) catches every folder-nested project the OAuth user + // can `projects.get`, including the ones we couldn't enumerate. + const broadQueryHits: string[] = []; + fetchMock.mockImplementation(async (url: string) => { + // Simulate 403 PERMISSION_DENIED → my prod code path returns + // the collected-so-far ([]) and logs a warning. + if (url.includes('v3/folders')) { + return foldersPage({ folders: [] }); + } + // Direct arm: org children. + if ( + url.includes('v1/projects') && + url.includes('parent.id%3A43356919874') + ) { + return gcpPage({ + projects: [ + { projectId: 'direct-1', name: 'Direct One', projectNumber: '100' }, + ], + }); + } + // Broad fallback query (parent.type:folder, NO parent.id). + if ( + url.includes('v1/projects') && + url.includes('parent.type%3Afolder') && + !/parent\.id%3A/.test(url) + ) { + broadQueryHits.push(url); + return gcpPage({ + projects: [ + { projectId: 'propperai-prod', name: 'Propper Prod', projectNumber: '200' }, + { projectId: 'propperai-demo', name: 'Propper Demo', projectNumber: '300' }, + ], + }); + } + throw new Error(`Unexpected URL: ${url}`); + }); + + const result = await service.detectProjectsForOrg('token', '43356919874'); + + const ids = result.map((p) => p.id).sort(); + expect(ids).toEqual(['direct-1', 'propperai-demo', 'propperai-prod']); + // The fallback fired exactly once. + expect(broadQueryHits).toHaveLength(1); + }); + + it('does NOT fire the broad-query fallback when folder enumeration succeeds (precise scoping preserved)', async () => { + // Multi-org safety: when v3/folders works as expected, we stick + // with the precise per-folder query so customers in multiple + // orgs do not see projects from orgs other than the one they + // selected (the original cubic P2 concern from PR #2899). + let broadFallbackFired = false; + fetchMock.mockImplementation(async (url: string) => { + if (url.includes('v3/folders') && url.includes('organizations%2Fhealthy-org')) { + return foldersPage({ folders: ['healthy-folder'] }); + } + if (url.includes('v3/folders')) { + return foldersPage({ folders: [] }); // no sub-folders + } + if (url.includes('parent.id%3Ahealthy-org') && !url.includes('parent.type%3Afolder')) { + return gcpPage({ projects: [] }); + } + if ( + url.includes('parent.type%3Afolder') && + url.includes('parent.id%3Ahealthy-folder') + ) { + return gcpPage({ + projects: [ + { projectId: 'scoped-prod', name: 'Scoped', projectNumber: '500' }, + ], + }); + } + // Catch-all: if the broad fallback fires, the test fails. + if ( + url.includes('parent.type%3Afolder') && + !/parent\.id%3A/.test(url) + ) { + broadFallbackFired = true; + return gcpPage({ projects: [] }); + } + throw new Error(`Unexpected URL: ${url}`); + }); + + const result = await service.detectProjectsForOrg('token', 'healthy-org'); + expect(result.map((p) => p.id)).toEqual(['scoped-prod']); + expect(broadFallbackFired).toBe(false); + }); + it('still returns direct-arm projects when the folder arm throws entirely (no-regression guarantee)', async () => { - // If GCP's v2/folders endpoint throws or returns 4xx, the folder + // If GCP's v3/folders endpoint throws or returns 4xx, the folder // arm collapses to [] gracefully — the direct arm still works // and we are at minimum no worse than prod. fetchMock.mockImplementation(async (url: string) => { - if (url.includes('v2/folders')) { - throw new Error('simulated v2/folders network failure'); + if (url.includes('v3/folders')) { + throw new Error('simulated v3/folders network failure'); } if (url.includes('parent.id%3A555')) { return gcpPage({ @@ -342,10 +439,10 @@ describe('GCPSecurityService — project detection', () => { if (url.includes('v1/projects') && url.includes('parent.id%3A666') && !url.includes('parent.type%3Afolder')) { throw new Error('simulated direct-arm failure'); } - if (url.includes('v2/folders') && url.includes('organizations%2F666')) { + if (url.includes('v3/folders') && url.includes('organizations%2F666')) { return foldersPage({ folders: ['folder-x'] }); } - if (url.includes('v2/folders')) return foldersPage({ folders: [] }); + if (url.includes('v3/folders')) return foldersPage({ folders: [] }); if (url.includes('v1/projects') && url.includes('parent.id%3Afolder-x')) { return gcpPage({ projects: [ @@ -379,12 +476,12 @@ describe('GCPSecurityService — project detection', () => { fetchMock.mockImplementation((url: string) => { if ( - url.includes('v2/folders') && + url.includes('v3/folders') && url.includes('organizations%2Fbig-org') ) { return Promise.resolve(foldersPage({ folders: folderIds })); } - if (url.includes('v2/folders')) { + if (url.includes('v3/folders')) { return Promise.resolve(foldersPage({ folders: [] })); // no sub-folders } if ( @@ -453,10 +550,10 @@ describe('GCPSecurityService — project detection', () => { // Two folders, the first one's project list throws. The second // one should still return its projects. fetchMock.mockImplementation(async (url: string) => { - if (url.includes('v2/folders') && url.includes('organizations%2F777')) { + if (url.includes('v3/folders') && url.includes('organizations%2F777')) { return foldersPage({ folders: ['bad-folder', 'good-folder'] }); } - if (url.includes('v2/folders')) return foldersPage({ folders: [] }); + if (url.includes('v3/folders')) return foldersPage({ folders: [] }); if (url.includes('v1/projects') && url.includes('parent.id%3A777') && !url.includes('parent.type%3Afolder')) { return gcpPage({ projects: [] }); } diff --git a/apps/api/src/cloud-security/providers/gcp-security.service.ts b/apps/api/src/cloud-security/providers/gcp-security.service.ts index 13160b9c0..e0b064062 100644 --- a/apps/api/src/cloud-security/providers/gcp-security.service.ts +++ b/apps/api/src/cloud-security/providers/gcp-security.service.ts @@ -889,7 +889,44 @@ export class GCPSecurityService { accessToken, organizationId, ); - if (folderIds.length === 0) return []; + + // Verified in production logs (customer Propper, org 43356919874): + // the `cloudresourcemanager.folders.list` endpoint returns + // `403 PERMISSION_DENIED` for some OAuth grants even when the + // user has `roles/owner` + `roles/resourcemanager.folderAdmin` + // at the org level. When that happens, folder enumeration + // returns an empty list and the precise per-folder query would + // silently skip every folder-nested project — exactly the bug + // Greg reported (propperai-prod, propperai-demo invisible in + // the picker). + // + // Fall back to the broader `parent.type:folder` query — it + // returns all folder-nested projects the OAuth user can `get`. + // For single-org tenants (the common case) this delivers the + // right set. For multi-org tenants it may include folder-nested + // projects from other orgs the user happens to have access to, + // which is acceptable because: + // - the picker is selection-based (user chooses what to monitor), + // - the alternative is a silently empty picker, + // - the user already authorized those projects via IAM. + if (folderIds.length === 0) { + this.logger.warn( + `GCP folder enumeration returned 0 folders for org ${organizationId}; falling back to broad parent.type:folder query`, + ); + const fallback = await this.listProjectsPaginated( + accessToken, + 'lifecycleState:ACTIVE AND parent.type:folder', + ).catch((err) => { + this.logger.warn( + `GCP broad folder-projects fallback failed: ${err instanceof Error ? err.message : String(err)}`, + ); + return []; + }); + this.logger.log( + `GCP folder fallback for org ${organizationId}: ${fallback.length} project(s) via broad parent.type:folder query`, + ); + return fallback; + } // Bound the fan-out: an unbounded `Promise.all(folderIds.map(...))` // can trigger GCP rate limiting (`429 Too Many Requests`) on @@ -955,10 +992,20 @@ export class GCPSecurityService { } /** - * One paginated call to `v2/folders?parent=` returning the + * One paginated call to `v3/folders?parent=` returning the * immediate child folder IDs (stripped of the "folders/" prefix). + * + * v3 was chosen over v2 because v2 is deprecated and was observed + * returning `403 PERMISSION_DENIED` for OAuth grants that + * legitimately had org-level folder permissions (verified in prod + * logs against customer Propper). v3 is the current API and + * accepts the same `parent`/`pageSize`/`pageToken` query params, + * so this swap is purely defensive — the response shape is + * identical. + * * Errors are non-fatal — log and return what we collected so far so - * one bad page doesn't kill the whole tree walk. + * one bad page doesn't kill the whole tree walk; the caller has a + * broad-query fallback when enumeration comes back empty. */ private async listChildFolders( accessToken: string, @@ -976,7 +1023,7 @@ export class GCPSecurityService { if (pageToken) params.set('pageToken', pageToken); const response = await fetch( - `https://cloudresourcemanager.googleapis.com/v2/folders?${params.toString()}`, + `https://cloudresourcemanager.googleapis.com/v3/folders?${params.toString()}`, { headers: { Authorization: `Bearer ${accessToken}`, From 173b031c47ed9ababe0559fc93d31015c2c20677 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Fri, 22 May 2026 14:36:40 -0400 Subject: [PATCH 2/2] fix(cloud-tests): only fire GCP broad fallback when folder list was forbidden MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic flagged PR #2916 (merged into main) with a P2 regression: the broad `parent.type:folder` fallback fired any time folder enumeration returned zero folders, including the case where an org legitimately has no folders at all. For multi-org tenants that meant the picker for an empty-folder org could surface folder-nested projects from OTHER orgs the OAuth user happened to have IAM access to (because the broad query is not org-scoped). Concrete cases: Case | Before (PR #2916) | After (this PR) ----------------------------------------------+-------------------+---------------- v3/folders returns 200 + empty (no folders) | Fallback fires | No fallback ✓ v3/folders returns 403 PERMISSION_DENIED | Fallback fires | Fallback fires ✓ v3/folders returns folder ids | No fallback | No fallback ✓ To distinguish the two cases, the folder-enumeration helpers now return `{ folderIds, forbidden }` where `forbidden` is true only when GCP responded with HTTP 403. The caller fires the broad query only when `folderIds.length === 0 && forbidden`. Customer Propper (Greg's case) is unaffected — their failure mode is "403 PERMISSION_DENIED" so the fallback still fires. New regression test locks in cubic's concern: when an org legitimately has no folders (200 OK + empty list), the broad fallback must NOT fire. The customer-Propper test was updated to return a real 403 response so it exercises the fallback path the way production does. Full cloud-security suite: 270/270 pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../providers/gcp-security.service.spec.ts | 96 +++++++++++++++---- .../providers/gcp-security.service.ts | 61 ++++++++---- 2 files changed, 121 insertions(+), 36 deletions(-) diff --git a/apps/api/src/cloud-security/providers/gcp-security.service.spec.ts b/apps/api/src/cloud-security/providers/gcp-security.service.spec.ts index 93df054bd..39fcc7c65 100644 --- a/apps/api/src/cloud-security/providers/gcp-security.service.spec.ts +++ b/apps/api/src/cloud-security/providers/gcp-security.service.spec.ts @@ -315,24 +315,31 @@ describe('GCPSecurityService — project detection', () => { expect(result).toEqual([]); }); - it('falls back to broad parent.type:folder query when folder enumeration returns empty (customer Propper regression)', async () => { - // Verified failure mode in production: v3/folders (and v2 before - // it) returns `403 PERMISSION_DENIED` for some OAuth grants - // despite the user having org-level folder roles. Folder - // enumeration silently returns []. The picker would have been - // missing folder-nested projects. - // - // After the fallback: a broad `parent.type:folder` query (no - // parent.id) catches every folder-nested project the OAuth user - // can `projects.get`, including the ones we couldn't enumerate. + it('falls back to broad parent.type:folder query when v3/folders returns 403 PERMISSION_DENIED (customer Propper regression)', async () => { + // Exact production failure mode: v3/folders returns + // `403 PERMISSION_DENIED` for some OAuth grants despite the + // user having org-level folder roles. The fallback MUST fire + // ONLY for true forbiddens — cubic P2 on PR #2916 noted that + // an unconditional fallback on "empty folders" would leak + // cross-org projects for multi-org users whose selected org + // simply has no folders. See the next test for that case. const broadQueryHits: string[] = []; fetchMock.mockImplementation(async (url: string) => { - // Simulate 403 PERMISSION_DENIED → my prod code path returns - // the collected-so-far ([]) and logs a warning. if (url.includes('v3/folders')) { - return foldersPage({ folders: [] }); + // Real 403 PERMISSION_DENIED body that GCP returns. + return { + ok: false, + status: 403, + text: async () => + JSON.stringify({ + error: { + code: 403, + message: 'The caller does not have permission', + status: 'PERMISSION_DENIED', + }, + }), + }; } - // Direct arm: org children. if ( url.includes('v1/projects') && url.includes('parent.id%3A43356919874') @@ -343,7 +350,6 @@ describe('GCPSecurityService — project detection', () => { ], }); } - // Broad fallback query (parent.type:folder, NO parent.id). if ( url.includes('v1/projects') && url.includes('parent.type%3Afolder') && @@ -364,10 +370,68 @@ describe('GCPSecurityService — project detection', () => { const ids = result.map((p) => p.id).sort(); expect(ids).toEqual(['direct-1', 'propperai-demo', 'propperai-prod']); - // The fallback fired exactly once. expect(broadQueryHits).toHaveLength(1); }); + it('does NOT fall back to the broad query when the org simply has no folders (cubic P2 fix)', async () => { + // The previous PR (#2916) fired the broad fallback for ANY + // empty enumeration result. That meant a multi-org user whose + // selected org legitimately had no folders would see folder- + // nested projects from OTHER orgs they had IAM access to — + // because the broad query is not org-scoped. + // + // This test locks in the cubic P2 fix: enumeration returns + // empty WITHOUT a 403 → no fallback → folder arm returns [] + // and the picker shows only the org's direct children. + const broadQueryHits: string[] = []; + fetchMock.mockImplementation(async (url: string) => { + if (url.includes('v3/folders')) { + // 200 OK + empty list. NOT a 403. The org just has no + // folders. Common shape for small single-org tenants too. + return foldersPage({ folders: [] }); + } + if ( + url.includes('v1/projects') && + url.includes('parent.id%3Aflat-org') + ) { + return gcpPage({ + projects: [ + { + projectId: 'only-direct', + name: 'Only Direct', + projectNumber: '900', + }, + ], + }); + } + if ( + url.includes('v1/projects') && + url.includes('parent.type%3Afolder') && + !/parent\.id%3A/.test(url) + ) { + // Would return cross-org projects if the fallback ever + // fires. Track hits — must be zero. + broadQueryHits.push(url); + return gcpPage({ + projects: [ + { + projectId: 'should-not-appear', + name: 'Other Org Folder Project', + projectNumber: '999', + }, + ], + }); + } + throw new Error(`Unexpected URL: ${url}`); + }); + + const result = await service.detectProjectsForOrg('token', 'flat-org'); + + expect(result.map((p) => p.id)).toEqual(['only-direct']); + // The broad fallback must NOT fire. Cubic P2. + expect(broadQueryHits).toHaveLength(0); + }); + it('does NOT fire the broad-query fallback when folder enumeration succeeds (precise scoping preserved)', async () => { // Multi-org safety: when v3/folders works as expected, we stick // with the precise per-folder query so customers in multiple diff --git a/apps/api/src/cloud-security/providers/gcp-security.service.ts b/apps/api/src/cloud-security/providers/gcp-security.service.ts index e0b064062..ba2a352ac 100644 --- a/apps/api/src/cloud-security/providers/gcp-security.service.ts +++ b/apps/api/src/cloud-security/providers/gcp-security.service.ts @@ -885,7 +885,7 @@ export class GCPSecurityService { accessToken: string, organizationId: string, ): Promise> { - const folderIds = await this.listFoldersUnderOrg( + const { folderIds, forbidden } = await this.listFoldersUnderOrg( accessToken, organizationId, ); @@ -900,18 +900,22 @@ export class GCPSecurityService { // Greg reported (propperai-prod, propperai-demo invisible in // the picker). // - // Fall back to the broader `parent.type:folder` query — it - // returns all folder-nested projects the OAuth user can `get`. - // For single-org tenants (the common case) this delivers the - // right set. For multi-org tenants it may include folder-nested - // projects from other orgs the user happens to have access to, - // which is acceptable because: - // - the picker is selection-based (user chooses what to monitor), - // - the alternative is a silently empty picker, - // - the user already authorized those projects via IAM. - if (folderIds.length === 0) { + // The fallback below ONLY fires when enumeration was actually + // forbidden, not when an org legitimately has zero folders. + // (cubic P2 on PR #2916: the previous always-on fallback could + // leak folder-nested projects from OTHER orgs in a multi-org + // tenant whose selected org simply had no folders.) + // + // Forbidden case → broad `parent.type:folder` query catches + // every folder-nested project the OAuth user can `projects.get`. + // For single-org tenants this delivers the right set. For + // multi-org tenants whose v3/folders is forbidden the fallback + // may include projects from other accessible orgs — acceptable + // because the picker is selection-based and the alternative is a + // silently empty picker. + if (folderIds.length === 0 && forbidden) { this.logger.warn( - `GCP folder enumeration returned 0 folders for org ${organizationId}; falling back to broad parent.type:folder query`, + `GCP folder enumeration was forbidden for org ${organizationId}; falling back to broad parent.type:folder query`, ); const fallback = await this.listProjectsPaginated( accessToken, @@ -928,6 +932,13 @@ export class GCPSecurityService { return fallback; } + if (folderIds.length === 0) { + // No folders exist for this org AND no permission errors were + // observed — return empty so a multi-org user doesn't see + // folder-nested projects from unrelated orgs. + return []; + } + // Bound the fan-out: an unbounded `Promise.all(folderIds.map(...))` // can trigger GCP rate limiting (`429 Too Many Requests`) on // tenants with many folders. Because `listProjectsPaginated` @@ -962,20 +973,25 @@ export class GCPSecurityService { private async listFoldersUnderOrg( accessToken: string, organizationId: string, - ): Promise { + ): Promise<{ folderIds: string[]; forbidden: boolean }> { const SAFE_MAX_FOLDERS = 500; const collected: string[] = []; const seenParents = new Set(); const queue: string[] = [`organizations/${organizationId}`]; + // Tracks whether ANY page in the BFS hit a 403 PERMISSION_DENIED. + // Only true forbiddens trigger the broad-query fallback in the + // caller; "no folders exist" must NOT trigger it. + let forbidden = false; while (queue.length > 0 && collected.length < SAFE_MAX_FOLDERS) { const parent = queue.shift(); if (!parent || seenParents.has(parent)) continue; seenParents.add(parent); - const children = await this.listChildFolders(accessToken, parent); - for (const folderId of children) { + const result = await this.listChildFolders(accessToken, parent); + if (result.forbidden) forbidden = true; + for (const folderId of result.folderIds) { if (collected.includes(folderId)) continue; collected.push(folderId); queue.push(`folders/${folderId}`); @@ -988,7 +1004,7 @@ export class GCPSecurityService { } } - return collected; + return { folderIds: collected, forbidden }; } /** @@ -1010,10 +1026,11 @@ export class GCPSecurityService { private async listChildFolders( accessToken: string, parent: string, - ): Promise { + ): Promise<{ folderIds: string[]; forbidden: boolean }> { const PAGE_SIZE = 100; const collected: string[] = []; let pageToken: string | undefined; + let forbidden = false; do { const params = new URLSearchParams({ @@ -1033,10 +1050,14 @@ export class GCPSecurityService { ); if (!response.ok) { + // Distinguish "forbidden" (the caller should fall back to a + // broader query) from "transient / no folders" (the caller + // should NOT broaden — empty results are the correct answer). + if (response.status === 403) forbidden = true; this.logger.warn( - `Failed to list child folders of ${parent}: ${await response.text()}`, + `Failed to list child folders of ${parent} (HTTP ${response.status}): ${await response.text()}`, ); - return collected; + return { folderIds: collected, forbidden }; } const data = (await response.json()) as { @@ -1052,7 +1073,7 @@ export class GCPSecurityService { pageToken = data.nextPageToken; } while (pageToken); - return collected; + return { folderIds: collected, forbidden }; } /**