apple testflight distribute: a build's groups read per group (GET /v1/betaGroups/{id}/relationships/builds) instead of GET /v1/betaGroups?filter[builds]=, which App Store Connect refuses with 400 although the spec declares it; internal all-builds groups reported as already available and never sent the build; a re-run adds nothing; configured groups missing on App Store Connect point at groups sync, unknown ones list what exists; APPLE_LIVE_REJECTED (refused by storebound api too), tests/asc-query-guard.test.ts checks every App Store Connect GET with a query in cli/ against the spec's declared parameters, and the fake App Store Connect answers 400 to undeclared or live-refused parameters (pitfall 42)
This commit is contained in:
parent
f741c15829
commit
bba46b5c7f
13 changed files with 354 additions and 14 deletions
163
tests/asc-query-guard.test.ts
Normal file
163
tests/asc-query-guard.test.ts
Normal file
|
|
@ -0,0 +1,163 @@
|
|||
// Offline guard for App Store Connect GETs (pitfall: a filter App Store Connect refuses fails only at run time, with a
|
||||
// 400 in the middle of a release). Walks every App Store Connect path with a query string that storebound's code builds
|
||||
// (template literals "/v1/...?...", including the URLSearchParams objects they interpolate), resolves it to the
|
||||
// operation in cli/api/apple.gen.ts (generated from Apple's OpenAPI spec) and checks every query parameter against the
|
||||
// operation's declared parameters and against APPLE_LIVE_REJECTED (declared, but refused live).
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { readdirSync, readFileSync, statSync } from 'node:fs';
|
||||
import { join, relative } from 'node:path';
|
||||
import { APPLE_LIVE_REJECTED, buildRequest, getOp, resolveOp } from '../cli/api/client';
|
||||
|
||||
const ROOT = join(import.meta.dir, '..');
|
||||
|
||||
/** Expressions that name a resource type in a path (`/v1/apps/${appId}/${kind}?...`), per file: every combination the
|
||||
* callers pass, so each one is checked as its own GET. A new caller with another type adds a row here. */
|
||||
const SEGMENT_VALUES: Record<string, Record<string, string>[]> = {
|
||||
'cli/triage.ts': [{ kind: 'betaFeedbackCrashSubmissions' }, { kind: 'betaFeedbackScreenshotSubmissions' }],
|
||||
'cli/apple/asc.ts': [{ 'o.set.type': 'appScreenshotSets', asset: 'appScreenshots' }, { 'o.set.type': 'appPreviewSets', asset: 'appPreviews' }],
|
||||
'cli/apple/screenshots.ts': [{ 'parent.type': 'appStoreVersionLocalizations' }, { 'parent.type': 'appCustomProductPageLocalizations' },
|
||||
{ 'parent.type': 'appStoreVersionExperimentTreatmentLocalizations' }],
|
||||
};
|
||||
|
||||
function files(dir: string): string[] {
|
||||
return readdirSync(dir).flatMap(n => {
|
||||
const p = join(dir, n);
|
||||
if (statSync(p).isDirectory()) return files(p);
|
||||
return /\.ts$/.test(n) && !/\.gen\.ts$/.test(n) ? [p] : [];
|
||||
});
|
||||
}
|
||||
|
||||
/** Index just after the string / template literal that opens at `i`. */
|
||||
function literalEnd(s: string, i: number): number {
|
||||
const q = s[i]!;
|
||||
for (let j = i + 1; j < s.length; j++) {
|
||||
const c = s[j];
|
||||
if (c === '\\') { j++; continue; }
|
||||
if (c === q) return j + 1;
|
||||
if (q === '`' && c === '$' && s[j + 1] === '{') j = exprEnd(s, j + 2) - 1;
|
||||
}
|
||||
return s.length;
|
||||
}
|
||||
|
||||
/** Index just after the `}` that closes an expression starting at `i`. */
|
||||
function exprEnd(s: string, i: number): number {
|
||||
let depth = 1;
|
||||
for (let j = i; j < s.length; j++) {
|
||||
const c = s[j]!;
|
||||
if (c === '\'' || c === '"' || c === '`') { j = literalEnd(s, j) - 1; continue; }
|
||||
if (c === '{') depth++;
|
||||
else if (c === '}' && --depth === 0) return j + 1;
|
||||
}
|
||||
return s.length;
|
||||
}
|
||||
|
||||
/** Top-level keys of an object literal's body ("'filter[app]': appId, include: 'x'"). */
|
||||
function objectKeys(body: string): string[] {
|
||||
const parts: string[] = [];
|
||||
let depth = 0, from = 0;
|
||||
for (let j = 0; j < body.length; j++) {
|
||||
const c = body[j]!;
|
||||
if (c === '\'' || c === '"' || c === '`') { j = literalEnd(body, j) - 1; continue; }
|
||||
if ('([{'.includes(c)) depth++;
|
||||
else if (')]}'.includes(c)) depth--;
|
||||
else if (c === ',' && depth === 0) { parts.push(body.slice(from, j)); from = j + 1; }
|
||||
}
|
||||
parts.push(body.slice(from));
|
||||
return parts.map(p => /^\s*(?:'([^']+)'|"([^"]+)"|([A-Za-z_$][\w$]*))\s*:/.exec(p)).filter(m => !!m).map(m => m![1] ?? m![2] ?? m![3]!);
|
||||
}
|
||||
|
||||
/** Keys of `new URLSearchParams({...})` starting at `i` (index of "new"). */
|
||||
function paramsObjectKeys(s: string, i: number): string[] {
|
||||
const open = s.indexOf('{', i);
|
||||
return objectKeys(s.slice(open + 1, exprEnd(s, open + 1) - 1));
|
||||
}
|
||||
|
||||
interface Site { file: string; line: number; path: string; keys: string[] }
|
||||
|
||||
function sites(): Site[] {
|
||||
const out: Site[] = [];
|
||||
for (const f of files(join(ROOT, 'cli'))) {
|
||||
const src = readFileSync(f, 'utf8');
|
||||
const rel = relative(ROOT, f);
|
||||
for (const m of src.matchAll(/[`'"](?=\/v[1-3]\/[^`'"\s]*\?)/g)) {
|
||||
const start = m.index!;
|
||||
const lit = src.slice(start + 1, literalEnd(src, start) - 1);
|
||||
const qAt = lit.indexOf('?');
|
||||
// the path: every ${...} is one segment (an id)
|
||||
const rawPath = lit.slice(0, qAt);
|
||||
const query = lit.slice(qAt + 1);
|
||||
const keys = [...query.matchAll(/(?:^|[&?])([A-Za-z]\w*(?:\[[\w.$]+\])?)=/g)].map(x => x[1]!);
|
||||
// fields[${kind}] style keys keep their ${...} for the segment values below
|
||||
keys.push(...[...query.matchAll(/(?:^|[&?])([A-Za-z]\w*\[\$\{\w+\}\])=/g)].map(x => x[1]!));
|
||||
// the whole query from a URLSearchParams: inline, or a variable defined before (plus its .set / .append calls)
|
||||
const whole = /^\$\{(.*)\}$/s.exec(query);
|
||||
if (whole) {
|
||||
const expr = whole[1]!.trim();
|
||||
if (expr.startsWith('new URLSearchParams(')) keys.push(...paramsObjectKeys(lit, lit.indexOf('new URLSearchParams(', qAt)));
|
||||
else {
|
||||
const v = /^(\w+)(?:\.toString\(\))?$/.exec(expr)?.[1];
|
||||
if (!v) throw new Error(`${rel}: cannot read the query of ${lit}`);
|
||||
const def = [...src.slice(0, start).matchAll(new RegExp(`(?:const|let)\\s+${v}\\s*=\\s*`, 'g'))].at(-1);
|
||||
if (!def) throw new Error(`${rel}: no definition of "${v}" before ${lit}`);
|
||||
const at = def.index! + def[0].length;
|
||||
if (/^new URLSearchParams\(\s*\{/.test(src.slice(at))) keys.push(...paramsObjectKeys(src, at));
|
||||
else if (/^new URLSearchParams\(\s*\)/.test(src.slice(at))) { /* filled by .set / .append below */ }
|
||||
else if (/^['"`]/.test(src.slice(at))) keys.push(...[...src.slice(at + 1, literalEnd(src, at) - 1).matchAll(/(?:^|&)([A-Za-z]\w*(?:\[[\w.]+\])?)=/g)].map(x => x[1]!));
|
||||
else throw new Error(`${rel}: cannot read the query "${v}" of ${lit}`);
|
||||
const between = src.slice(def.index!, start);
|
||||
keys.push(...[...between.matchAll(new RegExp(`\\b${v}\\.(?:set|append)\\(\\s*'([^']+)'`, 'g'))].map(x => x[1]!));
|
||||
}
|
||||
}
|
||||
const line = src.slice(0, start).split('\n').length;
|
||||
const combos = (SEGMENT_VALUES[rel] ?? []).filter(c => Object.keys(c).some(n => rawPath.includes(`\${${n}}`)));
|
||||
for (const sub of combos.length ? combos : [{}]) {
|
||||
const fill = (t: string) => Object.entries(sub).reduce((acc, [n, v]) => acc.split(`\${${n}}`).join(v), t);
|
||||
const path = fill(rawPath).replace(/\$\{[^}]*\}/g, 'X');
|
||||
out.push({ file: rel, line, path, keys: [...new Set(keys.map(fill))] });
|
||||
}
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
describe('App Store Connect GETs built by storebound', () => {
|
||||
const all = sites();
|
||||
|
||||
test('the walk finds the GETs (a broken scanner must not pass by finding nothing)', () => {
|
||||
expect(all.length).toBeGreaterThan(60);
|
||||
expect(all.some(s => s.file === 'cli/apple/testflight.ts' && s.path === '/v1/builds' && s.keys.includes('filter[preReleaseVersion.platform]'))).toBe(true);
|
||||
expect(all.some(s => s.file === 'cli/lib/reports.ts' && s.path === '/v1/financeReports' && s.keys.includes('filter[regionCode]'))).toBe(true);
|
||||
});
|
||||
|
||||
test('every path is a declared GET and every query parameter is declared for it (and not refused live)', () => {
|
||||
const problems: string[] = [];
|
||||
for (const s of all) {
|
||||
let op;
|
||||
try { op = resolveOp('apple', 'GET', s.path).op; } catch (e) { problems.push(`${s.file}:${s.line} ${s.path}: ${(e as Error).message}`); continue; }
|
||||
const declared = new Set(op.query.map(q => q.name));
|
||||
for (const k of s.keys) {
|
||||
if (!declared.has(k)) problems.push(`${s.file}:${s.line} GET ${s.path} (${op.id}): "${k}" is not a parameter of the operation`);
|
||||
else if (APPLE_LIVE_REJECTED[op.id]?.[k]) problems.push(`${s.file}:${s.line} GET ${s.path} (${op.id}): "${k}" is refused live: ${APPLE_LIVE_REJECTED[op.id]![k]}`);
|
||||
}
|
||||
}
|
||||
expect(problems).toEqual([]);
|
||||
});
|
||||
|
||||
test('the old distribute lookup would have been caught', () => {
|
||||
const op = resolveOp('apple', 'GET', '/v1/betaGroups').op;
|
||||
expect(op.id).toBe('betaGroups_getCollection');
|
||||
expect(APPLE_LIVE_REJECTED[op.id]?.['filter[builds]']).toBeTruthy();
|
||||
});
|
||||
|
||||
test('every live-refused parameter names a declared one (the list stays honest when the spec moves)', () => {
|
||||
for (const [id, params] of Object.entries(APPLE_LIVE_REJECTED)) {
|
||||
const op = getOp('apple', id);
|
||||
expect(op).toBeDefined();
|
||||
for (const k of Object.keys(params)) expect(op!.query.map(q => q.name)).toContain(k);
|
||||
}
|
||||
});
|
||||
|
||||
test('storebound api refuses a live-refused parameter', () => {
|
||||
expect(() => buildRequest(getOp('apple', 'betaGroups_getCollection')!, { 'filter[builds]': 'b1' })).toThrow('refuses "filter[builds]"');
|
||||
});
|
||||
});
|
||||
|
|
@ -5,6 +5,7 @@
|
|||
// unclear answer; a 5xx before it; a failing upload part), and new resources can be hidden from list reads for a
|
||||
// while (the stores' list lag right after a create). Nothing here reaches the network.
|
||||
import { json, type Call } from './store-mock';
|
||||
import { APPLE_LIVE_REJECTED, resolveOp } from '../../cli/api/client';
|
||||
|
||||
export type R = { type: string; id: string; attributes: Record<string, unknown>; rels: Record<string, string | string[]>; order: number; hiddenReads?: number };
|
||||
export interface Fault { method: string; match: RegExp; when: 'before' | 'after'; status?: number; times?: number; code?: string; detail?: string }
|
||||
|
|
@ -196,11 +197,30 @@ export class FakeAsc {
|
|||
return r;
|
||||
}
|
||||
|
||||
/**
|
||||
* App Store Connect answers 400 PARAMETER_ERROR.ILLEGAL to a query parameter the operation does not take, and (seen
|
||||
* live) to some the spec declares (APPLE_LIVE_REJECTED: filter[builds] on GET /v1/betaGroups). The fake does the same,
|
||||
* so a command that builds such a GET fails here as it fails live.
|
||||
*/
|
||||
illegalQuery(u: URL): Response | null {
|
||||
let op;
|
||||
try { op = resolveOp('apple', 'GET', u.pathname).op; } catch { return null; }
|
||||
const declared = new Set(op.query.map(q => q.name));
|
||||
for (const k of new Set(u.searchParams.keys())) {
|
||||
if (declared.has(k) && !APPLE_LIVE_REJECTED[op.id]?.[k]) continue;
|
||||
return json({ errors: [{ id: '00000000-0000-0000-0000-000000000000', status: '400', code: 'PARAMETER_ERROR.ILLEGAL', title: 'A parameter has an invalid value',
|
||||
detail: `'${k}' is not a valid ${/^filter\[/.test(k) ? 'filter type' : 'parameter'}`, source: { parameter: k } }] }, 400);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
handle(method: string, path: string, body: unknown): Response {
|
||||
const u = new URL(ASC + path);
|
||||
const parts = u.pathname.split('/').filter(Boolean); // v1, type, id?, rel?
|
||||
const [, type, id, rel, rel2] = parts;
|
||||
if (method === 'GET') {
|
||||
const bad = this.illegalQuery(u);
|
||||
if (bad) return bad;
|
||||
if (type === 'territories') return json({ data: this.all('territories').map(r => this.view(r)), links: {} });
|
||||
if (rel === 'pricePoints') {
|
||||
const t = u.searchParams.get('filter[territory]') ?? 'USA';
|
||||
|
|
@ -232,7 +252,24 @@ export class FakeAsc {
|
|||
return json({ data: shown.map(r => this.view(r)), included, links: {} });
|
||||
}
|
||||
if (method === 'POST') {
|
||||
if (rel === 'relationships') { return new Response(null, { status: 204 }); }
|
||||
if (rel === 'relationships') {
|
||||
const self = id ? this.res.get(id) : undefined;
|
||||
const items = ((body as { data?: { id: string }[] } | undefined)?.data ?? []);
|
||||
// App Store Connect gives an internal all-builds group every build by itself and refuses adding one by hand
|
||||
// (the exact answer is not recorded; a 409 stands for it)
|
||||
if (self?.type === 'betaGroups' && rel2 === 'builds' && self.attributes.isInternalGroup === true && self.attributes.hasAccessToAllBuilds === true)
|
||||
return json({ errors: [{ status: '409', code: 'STATE_ERROR', title: 'The request cannot be fulfilled because of the state of another resource.', detail: 'internal group with access to all builds' }] }, 409);
|
||||
// the link is kept: the item points back at the parent, so GET .../relationships/<rel> and the item's own
|
||||
// relationships list it (a build in a beta group, a tester in a group)
|
||||
if (self) for (const it of items) {
|
||||
const child = this.res.get(it.id);
|
||||
if (!child) continue;
|
||||
const cur = child.rels[type!];
|
||||
const list = cur === undefined ? [] : Array.isArray(cur) ? cur : [cur];
|
||||
if (!list.includes(self.id)) child.rels[type!] = [...list, self.id];
|
||||
}
|
||||
return new Response(null, { status: 204 });
|
||||
}
|
||||
const out = this.create(body as Parameters<FakeAsc['create']>[0]);
|
||||
return out instanceof Response ? out : json({ data: this.view(out) }, 201);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -98,6 +98,53 @@ describe('apple testflight groups: bare lists, sync creates', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('apple testflight distribute (real use 2026-10-13)', () => {
|
||||
const TF: ShipConfig = { ...CONFIG, apple: { ...CONFIG.apple!, testflight: { groups: [{ name: 'Team', internal: true, allBuilds: true }, { name: 'Friends' }, { name: 'Devs', internal: true }] } as never } };
|
||||
const groupsAndBuild = () => {
|
||||
s.asc.put({ type: 'betaGroups', id: 'grp-team', attributes: { name: 'Team', isInternalGroup: true, hasAccessToAllBuilds: true }, rels: { app: APP } });
|
||||
s.asc.put({ type: 'betaGroups', id: 'grp-friends', attributes: { name: 'Friends', isInternalGroup: false }, rels: { app: APP } });
|
||||
s.asc.put({ type: 'betaGroups', id: 'grp-devs', attributes: { name: 'Devs', isInternalGroup: true, hasAccessToAllBuilds: false }, rels: { app: APP } });
|
||||
s.asc.put({ type: 'builds', id: 'b-1', attributes: { version: '1', processingState: 'VALID', usesNonExemptEncryption: false, expired: false }, rels: { app: APP } });
|
||||
};
|
||||
|
||||
test('live shape: GET /v1/betaGroups?filter[builds]= is a 400 (the old lookup fails here as it did live)', async () => {
|
||||
const r = s.asc.handle('GET', `/v1/betaGroups?filter[app]=${APP}&filter[builds]=b-1&limit=200`, undefined);
|
||||
expect(r.status).toBe(400);
|
||||
expect(JSON.stringify(await r.json())).toContain('PARAMETER_ERROR.ILLEGAL');
|
||||
});
|
||||
|
||||
test('an internal all-builds group: reported as already available, nothing added', async () => {
|
||||
groupsAndBuild();
|
||||
const p = project(TF);
|
||||
const r = await twice(() => appleTestflight(p.ctx(['distribute', '--build', '1', '--group', 'Team', '--yes'])));
|
||||
expect(r.first).toEqual([]);
|
||||
expect(r.second).toEqual([]);
|
||||
const out = await capture(() => appleTestflight(p.ctx(['distribute', '--build', '1', '--group', 'Team', '--yes'])));
|
||||
expect(out.out).toContain('already available to Team (internal, all builds)');
|
||||
expect(s.calls.some(c => /filter\[builds\]/.test(decodeURIComponent(c.path)))).toBe(false);
|
||||
});
|
||||
|
||||
test('an internal group without all builds and an external group: added once, the re-run adds nothing', async () => {
|
||||
groupsAndBuild();
|
||||
const p = project(TF);
|
||||
const r = await twice(() => appleTestflight(p.ctx(['distribute', '--build', '1', '--group', 'Team,Devs,Friends', '--yes'])));
|
||||
expect(r.first).toEqual(['POST /v1/betaGroups/grp-devs/relationships/builds', 'POST /v1/betaGroups/grp-friends/relationships/builds']);
|
||||
expect(r.second).toEqual([]);
|
||||
const out = await capture(() => appleTestflight(p.ctx(['distribute', '--build', '1', '--group', 'Devs', '--yes'])));
|
||||
expect(out.out).toContain('already in Devs (internal)');
|
||||
});
|
||||
|
||||
test('a group in the config but not on App Store Connect points at groups sync; an unknown one lists what exists', async () => {
|
||||
const p = project(TF);
|
||||
s.asc.put({ type: 'builds', id: 'b-1', attributes: { version: '1', processingState: 'VALID' }, rels: { app: APP } });
|
||||
await expect(capture(() => appleTestflight(p.ctx(['distribute', '--build', '1', '--group', 'Team', '--yes']))))
|
||||
.rejects.toThrow('group "Team" is in the config but not on App Store Connect yet: create it with `storebound apple testflight groups sync`');
|
||||
s.asc.put({ type: 'betaGroups', id: 'grp-friends', attributes: { name: 'Friends', isInternalGroup: false }, rels: { app: APP } });
|
||||
await expect(capture(() => appleTestflight(p.ctx(['distribute', '--build', '1', '--group', 'Nope', '--yes']))))
|
||||
.rejects.toThrow('unknown group "Nope": App Store Connect has "Friends"');
|
||||
});
|
||||
});
|
||||
|
||||
describe('App Store Connect: a second run writes nothing', () => {
|
||||
test('apple iap: product, localizations, price schedule, availability once', async () => {
|
||||
const p = project(CONFIG);
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import { parseCsv, parseCsvRecords, toCsv } from '../cli/lib/csv';
|
|||
import { validateConfig } from '../cli/lib/config';
|
||||
import {
|
||||
betaDescriptions, betaReadiness, betaReviewReq, buildsWithVersion, type Group, groupFromApi, inviteReq, maskEmail, parseTesters, pickBuild,
|
||||
planBetaInfo, planBetaReviewDetail, planDistribute, planGroups, planTesters, planWhatToTest, validateTestflight, whatToTestFromListing,
|
||||
planBetaInfo, planBetaReviewDetail, planDistribute, planGroups, planTesters, planWhatToTest, unknownGroupMessage, validateTestflight, whatToTestFromListing,
|
||||
} from '../cli/lib/testflight';
|
||||
import { TOOLS, toArgv, validateInput } from '../cli/mcp-tools';
|
||||
|
||||
|
|
@ -103,6 +103,13 @@ describe('testflight builds', () => {
|
|||
expect(betaReadiness({ processingState: 'VALID', expired: true }, null)[0]).toContain('expired');
|
||||
expect(betaReadiness({ processingState: 'VALID', usesNonExemptEncryption: null }, { internalBuildState: 'MISSING_EXPORT_COMPLIANCE' })[0]).toContain('export compliance');
|
||||
});
|
||||
test('unknown groups: configured ones point at groups sync, others list what exists', () => {
|
||||
const cfg = [{ name: 'Team', internal: true }, { name: 'QA' }];
|
||||
expect(unknownGroupMessage(['Team'], [], cfg)).toBe('group "Team" is in the config but not on App Store Connect yet: create it with `storebound apple testflight groups sync`');
|
||||
expect(unknownGroupMessage(['Team', 'QA'], [], cfg)).toContain('groups "Team", "QA" are in the config');
|
||||
expect(unknownGroupMessage(['Nope'], G, cfg)).toBe('unknown group "Nope": App Store Connect has "Team", "Everyone inside", "Friends"');
|
||||
expect(unknownGroupMessage(['Nope'], [], cfg)).toContain('no beta groups yet');
|
||||
});
|
||||
test('distribute skips groups that have it and internal all-builds groups', () => {
|
||||
expect(planDistribute('b1', G, ['g-ext']).map(r => r.path)).toEqual(['/v1/betaGroups/g-int/relationships/builds']);
|
||||
expect(planDistribute('b1', G, [])[0]!.body).toEqual({ data: [{ type: 'builds', id: 'b1' }] });
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue