Skip to content

Commit d374f61

Browse files
fix(collect): redact rejected URL diagnostics
what changed: Fix #378; mcp.mjs, collect-remote.mjs, MCP/CLI regression coverage, architecture map, changelog and test inventory. why: Restore the existing collection input and credential-handling contract. This is a bug fix with one remedy; no alternative design was set aside. what it touched: Shared remote collection seams; existing ADR-0033/0034 behavior. No new domain term or public interface. what you verified: verified - dispatch/MCP red-first reproduction and passing combined regression run (83 tests); CLI integration regressions pass in the broader command run. The commit gate runs the required complete offline suite before accepting this commit. what you got wrong and fixed: mistake - running six full suites concurrently exceeded the default 3900-second budget; where - local commit validation on this Windows host; impact - affected hooks refused commits before publication; cause - excess concurrent subprocess and filesystem work; fix - retry one commit at a time with the ADR-0142 timeout extension (RESEARCH_KIT_GATE_TIMEOUT=7920); verified - the required full-suite commit gate accepted the commit.
1 parent 3fa03fa commit d374f61

7 files changed

Lines changed: 32 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ Each release of Research-Kit, newest first. The version is `KIT_VERSION` in
44
`research-kit/lib/core.mjs`; a release is tagged `v<version>` (ADR-0116).
55

66
## Unreleased
7+
- Rejected collection URLs pass through the existing credential redactor before
8+
MCP tool results or `collect-remote` stderr can echo them (#378). Regression
9+
coverage includes caller credentials in URL userinfo and query parameters.
10+
711
- Fixed `AGENTS.md` corpus description to use evergreen relative references rather than stale hardcoded entry counts (#299).
812

913
- The suite's "every CLI refuses a flag it does not know" table now covers `measure`,

‎docs/ARCHITECTURE.md‎

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.

‎research-kit/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ node research-kit/bin/selftest.mjs # all of it
563563
node research-kit/bin/selftest.mjs gate hook # just these files
564564
```
565565

566-
1789 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means
566+
1791 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means
567567
the assertions settled (ADR-0021), and each test is raced against a watchdog
568568
(`RESEARCH_KIT_TEST_TIMEOUT`, default 60s).
569569

‎research-kit/bin/collect-remote.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ checkFlagValues(flags, {
155155
// token would otherwise hide it, and a dispatched bad URL would be searched as text.
156156
const queriesValue = queriesInput({ queries: flagList(flags.query), urls: flagList(flags.url) });
157157
if (queriesValue.error) {
158-
process.stderr.write(`${queriesValue.error}\n fix: --url takes a page address starting with https://\n`);
158+
process.stderr.write(`${redact(queriesValue.error)}\n fix: --url takes a page address starting with https://\n`);
159159
process.exit(EXIT.CANNOT_START);
160160
}
161161

‎research-kit/lib/mcp.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ async function callTool(message, deps) {
311311
try {
312312
if (name === 'collect') {
313313
const queries = queriesInput({ queries: args.queries ?? [], urls: args.urls ?? [] });
314-
if (queries.error) return ok(message.id, { isError: true, content: [text(`${queries.error}. urls takes page addresses starting with https://`)] });
314+
if (queries.error) return ok(message.id, { isError: true, content: [text(redact(`${queries.error}. urls takes page addresses starting with https://`))] });
315315
const started = await deps.dispatch({
316316
repository: args.repository,
317317
inputs: {

‎research-kit/test/cli.test.mjs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,3 +1581,13 @@ test('install-hooks --dry-run answers what the real run answers, deployed or not
15811581
const gitconfig = fs.existsSync(path.join(home, 'gitconfig')) ? fs.readFileSync(path.join(home, 'gitconfig'), 'utf8') : '';
15821582
assert.doesNotMatch(gitconfig, /hooksPath/, 'a dry run set core.hooksPath');
15831583
});
1584+
1585+
test('collect-remote redacts credentials in refused URLs', () => {
1586+
const credentials = [`ghp_${'z'.repeat(36)}`, `github_pat_${'B2'.repeat(20)}`];
1587+
const r = run('collect-remote.mjs', ['--repository', 'o/r', '--topic', 't',
1588+
'--url', `ftp://user:${credentials[0]}@a.example/`, '--url', `ftp://a.example/?token=${credentials[1]}`], { root: project() });
1589+
assert.equal(r.status, 3);
1590+
for (const credential of credentials) assert.ok(!r.all.includes(credential), 'a caller credential leaked');
1591+
assert.match(r.err, /redacted/);
1592+
assert.match(r.err, /not an http\(s\) URL/);
1593+
});

‎research-kit/test/mcp.test.mjs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,3 +725,16 @@ test('the server installs the closed-stdout guard before it writes its first byt
725725
assert.ok(call !== -1, 'mcp-server.mjs no longer tolerates a closed stdout (2026-09-30)');
726726
assert.ok(call < firstWrite, `the guard is installed after the first write (write at ${firstWrite}, guard at ${call})`);
727727
});
728+
729+
test('refused URLs never echo caller credentials in a tool result', async () => {
730+
const credentials = [`ghp_${'z'.repeat(36)}`, `github_pat_${'B2'.repeat(20)}`];
731+
const d = deps();
732+
const r = await handle(call('collect', { repository: 'o/r', topic: 'x', urls: [
733+
`ftp://user:${credentials[0]}@a.example/`, `ftp://a.example/?token=${credentials[1]}`,
734+
] }), d);
735+
assert.equal(r.result.isError, true);
736+
for (const credential of credentials) assert.ok(!JSON.stringify(r).includes(credential), 'a caller credential leaked');
737+
assert.match(r.result.content[0].text, /redacted/);
738+
assert.match(r.result.content[0].text, /not an http\(s\) URL/);
739+
assert.equal(d.seen.dispatch, undefined, 'a refused URL must not start a run');
740+
});

0 commit comments

Comments
 (0)