Skip to content

Commit 2f86087

Browse files
fix(features): single source of truth for OAuth scopes; dedup read scopes (#323) (#347)
* fix(features): single source of truth for OAuth scopes; dedup read scopes when paired write enabled (#323) - `setup-gcp.sh` now sources scopes from `feature-config.ts` via a small `scripts/print-scopes.ts`, eliminating the hand-maintained list that drifted and caused users to hit "This app is blocked." - `resolveFeatures` skips a read group's scopes when its paired write group is enabled (the write scope grants read access at the API level). Avoids redundant `drive` + `drive.readonly` (etc.) consent prompts. - Drift-guard tests assert `print-scopes.ts` emits `getAllPossibleScopes()` and that `setup-gcp.sh` has no inlined scope list. * test: use execSync for cross-platform npx resolution in drift-guard execFileSync('npx', ...) fails on Windows because npx is npx.cmd and Node's spawn doesn't auto-resolve .cmd extensions without a shell. execSync runs through the shell on every platform. * fix(setup-gcp): filter scope output to https:// lines Address review feedback: ts-node stderr (Node deprecation warnings, etc.) was captured into SCOPES_OUTPUT via 2>&1 and would have been surfaced to the user as bogus OAuth scopes. Filter on the https:// prefix instead, and fail loudly if filtering leaves zero scopes.
1 parent c0a63f4 commit 2f86087

7 files changed

Lines changed: 211 additions & 19 deletions

File tree

scripts/print-scopes.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @license
3+
* Copyright 2026 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
/**
8+
* Prints the OAuth scopes that should be registered on the GCP consent
9+
* screen, one per line. Sourced from FEATURE_GROUPS so the registration
10+
* list and the runtime request list cannot drift.
11+
*
12+
* Used by scripts/setup-gcp.sh.
13+
*/
14+
15+
import { getAllPossibleScopes } from '../workspace-server/src/features/feature-config';
16+
17+
for (const scope of getAllPossibleScopes()) {
18+
console.log(scope);
19+
}

scripts/setup-gcp.sh

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -91,19 +91,31 @@ echo -e " 4. Under ${GREEN}Test users${NC}, add the email addresses of anyone"
9191
echo -e " who will use this extension (required while in Testing mode)"
9292
echo ""
9393

94-
SCOPES=(
95-
"https://www.googleapis.com/auth/documents"
96-
"https://www.googleapis.com/auth/drive"
97-
"https://www.googleapis.com/auth/calendar"
98-
"https://www.googleapis.com/auth/chat.spaces"
99-
"https://www.googleapis.com/auth/chat.messages"
100-
"https://www.googleapis.com/auth/chat.memberships"
101-
"https://www.googleapis.com/auth/userinfo.profile"
102-
"https://www.googleapis.com/auth/gmail.modify"
103-
"https://www.googleapis.com/auth/directory.readonly"
104-
"https://www.googleapis.com/auth/presentations.readonly"
105-
"https://www.googleapis.com/auth/spreadsheets.readonly"
106-
)
94+
# Single source of truth: scopes are computed from FEATURE_GROUPS in
95+
# workspace-server/src/features/feature-config.ts. See issue #323.
96+
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
97+
REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
98+
SCOPES_OUTPUT=$(cd "$REPO_ROOT" && npx --no-install ts-node --transpile-only scripts/print-scopes.ts 2>&1)
99+
if [ $? -ne 0 ]; then
100+
echo -e "${RED}Error: Failed to compute OAuth scopes from feature-config.ts.${NC}"
101+
echo -e "${RED}Did you run 'npm install' at the repo root?${NC}"
102+
echo "$SCOPES_OUTPUT"
103+
exit 1
104+
fi
105+
106+
SCOPES=()
107+
# Filter to https:// lines so any Node/ts-node warnings written to stderr
108+
# (captured via 2>&1 above so we can surface them on failure) don't end up
109+
# in the user-visible scope list.
110+
while IFS= read -r line; do
111+
[[ "$line" == https://* ]] && SCOPES+=("$line")
112+
done <<< "$SCOPES_OUTPUT"
113+
114+
if [ ${#SCOPES[@]} -eq 0 ]; then
115+
echo -e "${RED}Error: print-scopes.ts produced no scope output.${NC}"
116+
echo "$SCOPES_OUTPUT"
117+
exit 1
118+
fi
107119

108120
for scope in "${SCOPES[@]}"; do
109121
echo -e " ${GREEN}$scope${NC}"

workspace-server/src/__tests__/features/feature-config.test.ts

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,16 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7+
import { execSync } from 'node:child_process';
8+
import { readFileSync } from 'node:fs';
9+
import { join } from 'node:path';
10+
711
import { describe, it, expect } from '@jest/globals';
8-
import { FEATURE_GROUPS, featureGroupKey } from '../../features/feature-config';
12+
import {
13+
FEATURE_GROUPS,
14+
featureGroupKey,
15+
getAllPossibleScopes,
16+
} from '../../features/feature-config';
917

1018
describe('feature-config', () => {
1119
it('should have unique feature group keys', () => {
@@ -57,3 +65,64 @@ describe('feature-config', () => {
5765
expect(timeRead!.scopes).toEqual([]);
5866
});
5967
});
68+
69+
describe('getAllPossibleScopes (issue #323)', () => {
70+
it('should include both write and readonly scopes for paired groups', () => {
71+
const scopes = getAllPossibleScopes();
72+
// Both must be registered on the consent screen — users may flip
73+
// <service>.write off, which causes <service>.readonly to be requested.
74+
expect(scopes).toContain('https://www.googleapis.com/auth/drive');
75+
expect(scopes).toContain('https://www.googleapis.com/auth/drive.readonly');
76+
expect(scopes).toContain('https://www.googleapis.com/auth/gmail.modify');
77+
expect(scopes).toContain('https://www.googleapis.com/auth/gmail.readonly');
78+
expect(scopes).toContain('https://www.googleapis.com/auth/calendar');
79+
expect(scopes).toContain(
80+
'https://www.googleapis.com/auth/calendar.readonly',
81+
);
82+
});
83+
84+
it('should exclude default-OFF group scopes that are not in any default-ON group', () => {
85+
const scopes = getAllPossibleScopes();
86+
// tasks.* are default-OFF and their scopes are not shared with any
87+
// default-ON group, so they shouldn't be in the registration list.
88+
expect(scopes).not.toContain('https://www.googleapis.com/auth/tasks');
89+
expect(scopes).not.toContain(
90+
'https://www.googleapis.com/auth/tasks.readonly',
91+
);
92+
});
93+
94+
it('should be sorted and deduplicated', () => {
95+
const scopes = getAllPossibleScopes();
96+
const sortedUnique = [...new Set(scopes)].sort();
97+
expect(scopes).toEqual(sortedUnique);
98+
});
99+
100+
it('print-scopes.ts should emit the same list (drift guard for setup-gcp.sh)', () => {
101+
// setup-gcp.sh shells out to scripts/print-scopes.ts; if this test
102+
// fails, the consent screen registration list will drift from
103+
// FEATURE_GROUPS — which is the bug in issue #323.
104+
const repoRoot = join(__dirname, '..', '..', '..', '..');
105+
// execSync (not execFileSync) so Windows can resolve npx.cmd via the
106+
// shell. Tests run on ubuntu/macos/windows.
107+
const output = execSync(
108+
'npx --no-install ts-node --transpile-only scripts/print-scopes.ts',
109+
{ cwd: repoRoot, encoding: 'utf8' },
110+
);
111+
const printed = output.trim().split(/\r?\n/);
112+
expect(printed).toEqual(getAllPossibleScopes());
113+
});
114+
115+
it('setup-gcp.sh should not contain a hardcoded SCOPES list (drift guard)', () => {
116+
// If someone re-inlines the list, this test catches it.
117+
const repoRoot = join(__dirname, '..', '..', '..', '..');
118+
const setupScript = readFileSync(
119+
join(repoRoot, 'scripts', 'setup-gcp.sh'),
120+
'utf8',
121+
);
122+
// A hardcoded list would have a literal scope URL inside SCOPES=( ... ).
123+
const hardcodedMatch = setupScript.match(
124+
/SCOPES=\(\s*"https:\/\/www\.googleapis\.com\//,
125+
);
126+
expect(hardcodedMatch).toBeNull();
127+
});
128+
});

workspace-server/src/__tests__/features/feature-resolver.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,4 +196,58 @@ describe('resolveFeatures', () => {
196196
const unique = new Set(requiredScopes);
197197
expect(requiredScopes.length).toBe(unique.size);
198198
});
199+
200+
describe('read/write scope dedup (issue #323)', () => {
201+
const READONLY_PAIRS: Array<[string, string]> = [
202+
['drive.readonly', 'drive'],
203+
['calendar.readonly', 'calendar'],
204+
['chat.spaces.readonly', 'chat.spaces'],
205+
['chat.messages.readonly', 'chat.messages'],
206+
['chat.memberships.readonly', 'chat.memberships'],
207+
['gmail.readonly', 'gmail.modify'],
208+
];
209+
const fullUrl = (s: string) => `https://www.googleapis.com/auth/${s}`;
210+
211+
it.each(READONLY_PAIRS)(
212+
'should not request %s when paired write scope is enabled (defaults)',
213+
(readonlyScope, writeScope) => {
214+
const { requiredScopes } = resolveFeatures();
215+
expect(requiredScopes).not.toContain(fullUrl(readonlyScope));
216+
expect(requiredScopes).toContain(fullUrl(writeScope));
217+
},
218+
);
219+
220+
it.each([
221+
['drive.write:off', 'drive.readonly'],
222+
['calendar.write:off', 'calendar.readonly'],
223+
['gmail.write:off', 'gmail.readonly'],
224+
['chat.write:off', 'chat.spaces.readonly'],
225+
])(
226+
'should request readonly scope when write group disabled (%s)',
227+
(override, readonlyScope) => {
228+
const { requiredScopes } = resolveFeatures(undefined, override);
229+
expect(requiredScopes).toContain(fullUrl(readonlyScope));
230+
},
231+
);
232+
233+
it('should not affect tool registration — read tools stay enabled when write is on', () => {
234+
const { enabledTools } = resolveFeatures();
235+
expect(enabledTools.has('drive.search')).toBe(true);
236+
expect(enabledTools.has('gmail.search')).toBe(true);
237+
expect(enabledTools.has('calendar.list')).toBe(true);
238+
expect(enabledTools.has('chat.listSpaces')).toBe(true);
239+
});
240+
241+
it('should still include read scopes for services without a write group (people)', () => {
242+
const { requiredScopes } = resolveFeatures();
243+
expect(requiredScopes).toContain(fullUrl('directory.readonly'));
244+
expect(requiredScopes).toContain(fullUrl('userinfo.profile'));
245+
});
246+
247+
it('should still include readonly scopes for default-OFF write groups (slides, sheets)', () => {
248+
const { requiredScopes } = resolveFeatures();
249+
expect(requiredScopes).toContain(fullUrl('presentations.readonly'));
250+
expect(requiredScopes).toContain(fullUrl('spreadsheets.readonly'));
251+
});
252+
});
199253
});

workspace-server/src/features/feature-config.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,3 +255,23 @@ export const FEATURE_GROUPS: readonly FeatureGroup[] = [
255255
defaultEnabled: false,
256256
},
257257
] as const satisfies readonly FeatureGroup[];
258+
259+
/**
260+
* Every scope that any default-enabled feature group could request.
261+
*
262+
* This is the registration list for the OAuth consent screen — broader than
263+
* the runtime request set, because users can disable individual write groups
264+
* via WORKSPACE_FEATURE_OVERRIDES, which causes the paired read group's
265+
* `.readonly` scope to be requested. Both must already be registered, or
266+
* unverified apps hit "This app is blocked."
267+
*
268+
* Returned sorted for stable diffs in `setup-gcp.sh`.
269+
*/
270+
export function getAllPossibleScopes(): string[] {
271+
const set = new Set<string>();
272+
for (const fg of FEATURE_GROUPS) {
273+
if (!fg.defaultEnabled) continue;
274+
for (const scope of fg.scopes) set.add(scope);
275+
}
276+
return [...set].sort();
277+
}

workspace-server/src/features/feature-resolver.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,17 +145,31 @@ export function resolveFeatures(
145145
}
146146
}
147147

148-
// Collect enabled tools and scopes
148+
// Collect enabled tools and scopes.
149+
//
150+
// Scope dedup: when a service's `.write` group is enabled alongside its
151+
// `.read` group, the write scope already grants read access at the API
152+
// level, so we skip the read group's scopes. Avoids prompting the user
153+
// for both `drive` and `drive.readonly` (and equivalents) on consent.
154+
// Tools are unaffected — read tools still get registered.
149155
const enabledTools = new Set<string>();
150156
const scopeSet = new Set<string>();
151157

152158
for (const fg of FEATURE_GROUPS) {
153159
const key = featureGroupKey(fg);
154160
if (!groupEnabled.get(key)) continue;
155161

156-
// Add scopes for this enabled group
157-
for (const scope of fg.scopes) {
158-
scopeSet.add(scope);
162+
const writeKey = `${fg.service}.write`;
163+
const subsumedByWrite =
164+
fg.group === 'read' &&
165+
writeKey !== key &&
166+
groupIndex.has(writeKey) &&
167+
groupEnabled.get(writeKey) === true;
168+
169+
if (!subsumedByWrite) {
170+
for (const scope of fg.scopes) {
171+
scopeSet.add(scope);
172+
}
159173
}
160174

161175
// Add tools (minus individually disabled ones)

workspace-server/src/features/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7-
export { FEATURE_GROUPS, featureGroupKey } from './feature-config';
7+
export {
8+
FEATURE_GROUPS,
9+
featureGroupKey,
10+
getAllPossibleScopes,
11+
} from './feature-config';
812
export type { FeatureGroup } from './feature-config';
913
export { resolveFeatures, parseOverrides } from './feature-resolver';
1014
export type { ResolvedFeatures } from './feature-resolver';

0 commit comments

Comments
 (0)