mirror of https://github.com/garrytan/gstack.git
test: coverage backfill from the ship review
The telemetry-strip invariant only validated the sed FALLBACK while the live jq path went unchecked — the jq del() lists are now held to the same every-emitted-field bar, plus a behavioral pipe-through. The context-bill nested-skill double-count fix gets a regression pin (a revert shipped green before). The windowsHide tripwire gains terminal-agent-control.ts — the exact file the fix commit names. The ios-qa revoke-by-token_id branch gets its negative case: unknown ids revoke nothing and leave live sessions alone.
This commit is contained in:
Garry Tan
parent
6f63bbd66c
commit
0c78fad2a8
|
|
@ -52,4 +52,13 @@ describe('windowsHide on Windows-reachable spawns (#1835)', () => {
|
|||
expect((perms.match(/'icacls'/g) || []).length).toBeGreaterThanOrEqual(3);
|
||||
expectHideNearEvery(perms, "'icacls'");
|
||||
});
|
||||
|
||||
test('terminal-agent respawn in terminal-agent-control.ts passes windowsHide', () => {
|
||||
// The CLI cold-start + v1.44 watchdog respawn path. On Windows it runs
|
||||
// through the Node polyfill (dist/bun-polyfill.cjs) whose host default is
|
||||
// the opposite of Bun's — a visible console window on every watchdog
|
||||
// respawn is the symptom when the flag is dropped. Wider window: the
|
||||
// spawn's options object carries the full env wiring before the flag.
|
||||
expectHideNearEvery(SRC('terminal-agent-control.ts'), '(Bun as any).spawn(', 700);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -158,6 +158,32 @@ describe('daemon — loopback listener', () => {
|
|||
expect(daemon.tokenStore.list().some(s => s.identity === 'revoke-by-id@example.com')).toBe(false);
|
||||
});
|
||||
|
||||
test('revoke with an unknown token_id revokes nothing and leaves live sessions untouched', async () => {
|
||||
const minted = daemon.tokenStore.mint({
|
||||
identity: 'unknown-id-survivor@example.com',
|
||||
capability: 'observe',
|
||||
origin: 'owner_granted',
|
||||
});
|
||||
if ('error' in minted) throw new Error(minted.error);
|
||||
try {
|
||||
// Well-formed (16 hex chars) but matches no session's salted hash.
|
||||
const revoke = await fetchWith('POST', `http://127.0.0.1:${daemon.loopbackPort}/auth/revoke`, {
|
||||
body: JSON.stringify({ token_id: '0'.repeat(16) }),
|
||||
});
|
||||
expect(revoke.status).toBe(200);
|
||||
expect(JSON.parse(revoke.bodyText).revoked).toBe(0);
|
||||
|
||||
// The minted session must survive: still in the token store...
|
||||
expect(daemon.tokenStore.list().some(s => s.identity === 'unknown-id-survivor@example.com')).toBe(true);
|
||||
// ...and still visible on the list endpoint.
|
||||
const list = await fetchWith('GET', `http://127.0.0.1:${daemon.loopbackPort}/auth/sessions`);
|
||||
const { sessions } = JSON.parse(list.bodyText) as { sessions: Array<Record<string, unknown>> };
|
||||
expect(sessions.some(s => s.identity === 'unknown-id-survivor@example.com')).toBe(true);
|
||||
} finally {
|
||||
daemon.tokenStore.revoke(minted.token);
|
||||
}
|
||||
});
|
||||
|
||||
test('healthz returns 200 with mode=loopback', async () => {
|
||||
const r = await fetchWith('GET', `http://127.0.0.1:${daemon.loopbackPort}/healthz`);
|
||||
expect(r.status).toBe(200);
|
||||
|
|
|
|||
|
|
@ -152,6 +152,40 @@ describe("upstream fix a: root-as-container + walkMd exclusions", () => {
|
|||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("totalMd: a container skill excludes nested child skills' bytes; the grand total sums per-skill with no overlap", () => {
|
||||
// Regression pin for the v1.63 double-count: totalMd on a skill dir that
|
||||
// CONTAINS other skill dirs (the gstack root wraps the whole tree) used to
|
||||
// swallow the children's .md bytes too, so the TOTAL line billed every
|
||||
// nested skill twice. A revert of the topSeg/SKILL.md skip in totalMd
|
||||
// (lib/context-bill.ts) must fail here.
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-nested-total-"));
|
||||
fs.writeFileSync(path.join(tmp, "SKILL.md"), "---\nname: parent\ndescription: p\n---\n# Parent\n");
|
||||
fs.writeFileSync(path.join(tmp, "NOTES.md"), "n".repeat(1_000));
|
||||
// A non-skill subdir (no SKILL.md) still belongs to the parent's total.
|
||||
fs.mkdirSync(path.join(tmp, "references"));
|
||||
fs.writeFileSync(path.join(tmp, "references", "GUIDE.md"), "g".repeat(2_000));
|
||||
// Nested child skill with a LARGE .md — the bytes a revert double-counts.
|
||||
fs.mkdirSync(path.join(tmp, "child"));
|
||||
fs.writeFileSync(path.join(tmp, "child", "SKILL.md"), "---\nname: child\ndescription: c\n---\n# Child\n");
|
||||
fs.writeFileSync(path.join(tmp, "child", "BIG.md"), "x".repeat(50_000));
|
||||
|
||||
const bill = buildBill(tmp);
|
||||
const parent = bill.skills.find((s) => s.name !== "child")!;
|
||||
const child = bill.skills.find((s) => s.name === "child")!;
|
||||
expect(bill.skills).toHaveLength(2);
|
||||
|
||||
const parentOwn =
|
||||
fileBytes(tmp, "SKILL.md") + fileBytes(tmp, "NOTES.md") + fileBytes(tmp, "references", "GUIDE.md");
|
||||
const childOwn = fileBytes(tmp, "child", "SKILL.md") + fileBytes(tmp, "child", "BIG.md");
|
||||
// Parent's total is its OWN files only — the child's 50KB is excluded.
|
||||
expect(parent.totalMdBytes).toBe(parentOwn);
|
||||
expect(child.totalMdBytes).toBe(childOwn);
|
||||
// Grand total = sum of per-skill figures, every byte billed exactly once.
|
||||
expect(bill.totals.totalMdBytes).toBe(parentOwn + childOwn);
|
||||
expect(bill.totals.totalMdBytes).toBe(parent.totalMdBytes + child.totalMdBytes);
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("walkMd skips node_modules and dot-directories", () => {
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "context-bill-walk-"));
|
||||
fs.writeFileSync(path.join(tmp, "real.md"), "x");
|
||||
|
|
|
|||
|
|
@ -12,14 +12,20 @@
|
|||
* (bin/gstack-telemetry-log)
|
||||
*
|
||||
* gstack-telemetry-sync MUST strip every one of those fields before the remote
|
||||
* POST (bin/gstack-telemetry-sync). This test enforces that contract three ways:
|
||||
* POST (bin/gstack-telemetry-sync). The script has TWO strip paths — jq del()
|
||||
* is PRIMARY (structural, escape-proof), sed is the jq-less fallback — and
|
||||
* this test enforces the contract on both:
|
||||
*
|
||||
* 1. Coverage — every repo/branch field the producers emit is also stripped.
|
||||
* Catches "added a new repo field, forgot to strip it" (the rename-to-_repo
|
||||
* landmine, or any future producer drift).
|
||||
* 2. Behavior — run the ACTUAL sed strip expressions from the sync script over
|
||||
* a sample event line and assert no repo/branch field survives, while benign
|
||||
* fields do. Catches a broken/edited regex, not just a missing line.
|
||||
* 1. Coverage — every repo/branch field the producers emit is also stripped,
|
||||
* by every jq del() list AND by the sed expressions. Catches "added a new
|
||||
* repo field, forgot to strip it" (the rename-to-_repo landmine, or any
|
||||
* future producer drift) on whichever path a machine takes.
|
||||
* 2. Behavior — run the ACTUAL jq expression and the ACTUAL sed strip
|
||||
* expressions from the sync script over a sample event line and assert no
|
||||
* repo/branch field survives, while benign fields do. Catches a
|
||||
* broken/edited filter, not just a missing line. The jq leg also pins the
|
||||
* malformed-line contract: a line jq can't parse is dropped, never
|
||||
* forwarded unstripped.
|
||||
* 3. Floor — the three known fields are always in the stripped set, so deleting
|
||||
* a strip rule fails CI even if a producer also stops emitting it.
|
||||
*/
|
||||
|
|
@ -45,6 +51,16 @@ function extractSedExprs(scriptText: string): string[] {
|
|||
return [...scriptText.matchAll(/-e\s+'(s\/[^']*)'/g)].map((m) => m[1]);
|
||||
}
|
||||
|
||||
/** Pull every `jq -c 'del(...)'` filter out of the sync script, verbatim. */
|
||||
function extractJqDelFilters(scriptText: string): string[] {
|
||||
return [...scriptText.matchAll(/jq -c '(del\([^']*\))'/g)].map((m) => m[1]);
|
||||
}
|
||||
|
||||
/** The JSON keys a jq del() filter removes, e.g. `del(._repo_slug, .repo)`. */
|
||||
function fieldsFromJqDel(filter: string): string[] {
|
||||
return [...filter.matchAll(/\.([A-Za-z_][A-Za-z0-9_]*)/g)].map((m) => m[1]);
|
||||
}
|
||||
|
||||
/** The JSON key a strip expression targets, e.g. `,"repo":"[^"]*"` -> `repo`. */
|
||||
function fieldFromSedExpr(expr: string): string | null {
|
||||
const m = expr.match(/,"([A-Za-z_][A-Za-z0-9_]*)":/);
|
||||
|
|
@ -73,6 +89,25 @@ describe('telemetry no-repo-identity-egress invariant', () => {
|
|||
const strippedFields = new Set(
|
||||
strippedRepoExprs.map(fieldFromSedExpr).filter((f): f is string => f !== null),
|
||||
);
|
||||
const jqFilters = extractJqDelFilters(syncText);
|
||||
|
||||
// Repo-identity fields the producers emit into the synced file — computed
|
||||
// once, asserted against BOTH strip paths (jq primary, sed fallback). Only
|
||||
// emission lines that target the synced file (skill-usage.jsonl) count: the
|
||||
// preamble appends directly; gstack-telemetry-log builds the synced event
|
||||
// with a `printf '{"v":1,...` line into $JSONL_FILE (= skill-usage.jsonl).
|
||||
const preambleSynced = fs
|
||||
.readFileSync(PREAMBLE, 'utf-8')
|
||||
.split('\n')
|
||||
.filter((l) => l.includes('skill-usage.jsonl'));
|
||||
const telLogSynced = fs
|
||||
.readFileSync(TEL_LOG, 'utf-8')
|
||||
.split('\n')
|
||||
.filter((l) => l.includes('"v":1') || l.includes('skill-usage'));
|
||||
const emitted = new Set<string>([
|
||||
...emittedRepoFields(preambleSynced),
|
||||
...emittedRepoFields(telLogSynced),
|
||||
]);
|
||||
|
||||
test('floor: the three known repo-identity fields are stripped', () => {
|
||||
for (const field of REPO_IDENTITY_FLOOR) {
|
||||
|
|
@ -80,33 +115,34 @@ describe('telemetry no-repo-identity-egress invariant', () => {
|
|||
}
|
||||
});
|
||||
|
||||
test('coverage: every repo/branch field the producers emit into skill-usage.jsonl is stripped', () => {
|
||||
// Only emission lines that target the synced file (skill-usage.jsonl). The
|
||||
// preamble appends directly; gstack-telemetry-log builds the synced event
|
||||
// with a `printf '{"v":1,...` line into $JSONL_FILE (= skill-usage.jsonl).
|
||||
const preambleSynced = fs
|
||||
.readFileSync(PREAMBLE, 'utf-8')
|
||||
.split('\n')
|
||||
.filter((l) => l.includes('skill-usage.jsonl'));
|
||||
const telLogSynced = fs
|
||||
.readFileSync(TEL_LOG, 'utf-8')
|
||||
.split('\n')
|
||||
.filter((l) => l.includes('"v":1') || l.includes('skill-usage'));
|
||||
const emitted = new Set<string>([
|
||||
...emittedRepoFields(preambleSynced),
|
||||
...emittedRepoFields(telLogSynced),
|
||||
]);
|
||||
test('coverage: every repo/branch field the producers emit into skill-usage.jsonl is stripped (sed fallback path)', () => {
|
||||
// The preamble must emit "repo" — guards against the test silently passing
|
||||
// because a regex stopped matching the producer.
|
||||
expect(emitted.has('repo')).toBe(true);
|
||||
for (const field of emitted) {
|
||||
expect(
|
||||
strippedFields.has(field),
|
||||
`producer emits repo-identity field "${field}" but gstack-telemetry-sync does not strip it (would leak to remote)`,
|
||||
`producer emits repo-identity field "${field}" but gstack-telemetry-sync's sed fallback does not strip it (would leak to remote)`,
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
test('coverage: every jq del() list (the PRIMARY strip path) covers every emitted repo-identity field', () => {
|
||||
// Both tiers run a del() filter; each must strip full repo identity on its
|
||||
// own — a machine only ever takes one branch.
|
||||
expect(jqFilters.length).toBeGreaterThanOrEqual(2);
|
||||
expect(emitted.has('repo')).toBe(true); // producer-regex canary, as above
|
||||
for (const filter of jqFilters) {
|
||||
const delFields = new Set(fieldsFromJqDel(filter));
|
||||
for (const field of [...emitted, ...REPO_IDENTITY_FLOOR]) {
|
||||
expect(
|
||||
delFields.has(field),
|
||||
`jq filter "${filter}" does not del repo-identity field "${field}" (primary strip path would leak it to remote)`,
|
||||
).toBe(true);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test('behavior: the real sed expressions remove repo identity, keep benign fields', () => {
|
||||
const sample =
|
||||
'{"v":1,"ts":"2026-06-02T00:00:00Z","skill":"design-shotgun",' +
|
||||
|
|
@ -134,4 +170,51 @@ describe('telemetry no-repo-identity-egress invariant', () => {
|
|||
expect(cleaned).toContain('"sessions":3');
|
||||
expect(cleaned).toContain('"ts":"2026-06-02T00:00:00Z"');
|
||||
});
|
||||
|
||||
test('behavior: the real jq del() filters strip repo identity and drop malformed lines', () => {
|
||||
if (!Bun.which('jq')) return; // jq-less machine: the sed-fallback behavior test above is the live path
|
||||
|
||||
const sample =
|
||||
'{"v":1,"ts":"2026-06-02T00:00:00Z","skill":"design-shotgun",' +
|
||||
'"repo":"my-secret-repo","_repo_slug":"acme-my-secret-repo","_branch":"feature-x",' +
|
||||
'"sessions":3,"installation_id":"abc123"}';
|
||||
|
||||
// The identified-tier filter (no installation_id in its del list) and the
|
||||
// anonymous-tier filter (installation_id included) — run each verbatim.
|
||||
const identified = jqFilters.find((f) => !f.includes('installation_id'));
|
||||
const anonymous = jqFilters.find((f) => f.includes('installation_id'));
|
||||
expect(identified).toBeTruthy();
|
||||
expect(anonymous).toBeTruthy();
|
||||
|
||||
const runJq = (filter: string, input: string) => {
|
||||
const out = spawnSync(['jq', '-c', filter], { stdin: Buffer.from(input) });
|
||||
return { exitCode: out.exitCode, stdout: out.stdout.toString().trim() };
|
||||
};
|
||||
|
||||
const id = runJq(identified!, sample);
|
||||
expect(id.exitCode).toBe(0);
|
||||
// No repo/branch identity survives, value or key.
|
||||
expect(id.stdout).not.toContain('my-secret-repo');
|
||||
expect(id.stdout).not.toContain('feature-x');
|
||||
expect(id.stdout).not.toContain('"repo"');
|
||||
expect(id.stdout).not.toContain('_repo_slug');
|
||||
expect(id.stdout).not.toContain('_branch');
|
||||
// Benign fields are untouched; identified tier keeps installation_id.
|
||||
expect(id.stdout).toContain('"skill":"design-shotgun"');
|
||||
expect(id.stdout).toContain('"sessions":3');
|
||||
expect(id.stdout).toContain('"installation_id":"abc123"');
|
||||
|
||||
// Anonymous tier additionally drops installation_id.
|
||||
const anon = runJq(anonymous!, sample);
|
||||
expect(anon.exitCode).toBe(0);
|
||||
expect(anon.stdout).not.toContain('installation_id');
|
||||
expect(anon.stdout).not.toContain('my-secret-repo');
|
||||
|
||||
// Malformed line: jq fails and emits nothing — the sync script's
|
||||
// `|| CLEAN=""` + `[ -z "$CLEAN" ] && continue` drops it, so bytes the
|
||||
// strip never touched are never forwarded.
|
||||
const bad = runJq(identified!, '{"v":1,"repo":"my-secret-repo"');
|
||||
expect(bad.exitCode).not.toBe(0);
|
||||
expect(bad.stdout).toBe('');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue