mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-07-10 04:20:23 +02:00
1ce0509fc7
## Summary
Hardens the local `github-artifacts` MCP helper
(`tools/mcp/github-artifacts/server.js`) so the `GITHUB_TOKEN` bearer
token is only ever sent to allowlisted GitHub API hosts. Previously the
same token was attached when downloading image and ZIP URLs extracted
from issue/PR Markdown, which could forward the token to any host
referenced in untrusted issue content.
This is repository MCP dev-tooling, not the shipped PowerToys runtime.
## What changed
- **New `auth.js`** centralizes token handling:
- `AUTH_ALLOWED_HOSTS = { api.github.com }`
- `shouldSendGitHubToken(url)` — true only for HTTPS + allowlisted host
- `headersForUrl(url, token, extra)` — attaches `Authorization` only
when the host is allowlisted
- **`server.js`** routes all three request sites (`fetchJson`,
`downloadBytes`, `downloadZipBytes`) through `headersForUrl`. Image/ZIP
URLs from issue content are now fetched **without** `Authorization`.
Removed the ZIP dual-attempt auth retry that previously sent the token
to untrusted hosts.
- **New `test-token-allowlist.js`** regression test (wired into `npm
test` / `npm run test:unit`).
## Verification
- Unit test: 13/13 pass — token attached for `api.github.com`, withheld
from arbitrary hosts, loopback, look-alike hostnames
(`api.github.com.attacker.com`), non-HTTPS, and
`*.githubusercontent.com`.
- Cross-origin redirects: confirmed Node's `fetch` (undici) strips
`Authorization` when a redirect leaves the origin.
- Functionality preserved (real public issues, real token): issue #25595
images (5/5 downloaded) and issue #39476 `PowerToysReport_*.zip`
(downloaded + extracted) both succeed without the token — GitHub serves
this content via public/signed URLs.
## Note
For private-repo attachments that would require an `Authorization`
header on a non-API host, the token is intentionally not sent (the
recommended tradeoff). PowerToys is public, so the helper's normal use
is unaffected.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
78 lines
2.7 KiB
JavaScript
78 lines
2.7 KiB
JavaScript
// Regression test for the GITHUB_TOKEN allowlist (security fix).
|
|
// Run with: node test-token-allowlist.js
|
|
//
|
|
// Proves that the bearer token is attached ONLY to allowlisted GitHub API hosts
|
|
// and is NEVER sent to arbitrary URLs extracted from untrusted issue content.
|
|
|
|
import assert from "node:assert/strict";
|
|
import { shouldSendGitHubToken, headersForUrl } from "./auth.js";
|
|
|
|
const TOKEN = "PT_MCP_TEST_FAKE_TOKEN";
|
|
|
|
let failures = 0;
|
|
function check(name, fn) {
|
|
try {
|
|
fn();
|
|
console.log(` ok - ${name}`);
|
|
} catch (err) {
|
|
failures++;
|
|
console.error(` FAIL - ${name}: ${err.message}`);
|
|
}
|
|
}
|
|
|
|
// Hosts that MUST receive the token.
|
|
const allowed = [
|
|
"https://api.github.com/repos/microsoft/PowerToys/issues/1",
|
|
"https://api.github.com/repos/microsoft/PowerToys/issues/1/comments?per_page=100&page=1"
|
|
];
|
|
|
|
// Hosts that MUST NOT receive the token (attacker-controlled or non-API hosts,
|
|
// including GitHub's own user-content hosts which never need the API token).
|
|
const denied = [
|
|
"http://127.0.0.1:8000/capture.png",
|
|
"http://127.0.0.1:8000/PowerToysReport.zip?download=1",
|
|
"https://evil.example.com/capture.png",
|
|
"https://evil.example.com/PowerToysReport.zip",
|
|
"http://api.github.com/repos/microsoft/PowerToys/issues/1", // not HTTPS
|
|
"https://api.github.com.attacker.com/x", // look-alike host
|
|
"https://objects.githubusercontent.com/some/asset.zip",
|
|
"https://user-images.githubusercontent.com/1/screenshot.png",
|
|
"not a url"
|
|
];
|
|
|
|
for (const url of allowed) {
|
|
check(`token sent to allowlisted host: ${url}`, () => {
|
|
assert.equal(shouldSendGitHubToken(url), true);
|
|
const h = headersForUrl(url, TOKEN);
|
|
assert.equal(h["Authorization"], `Bearer ${TOKEN}`);
|
|
assert.equal(h["User-Agent"], "issue-images-mcp");
|
|
});
|
|
}
|
|
|
|
for (const url of denied) {
|
|
check(`token withheld from untrusted host: ${url}`, () => {
|
|
assert.equal(shouldSendGitHubToken(url), false);
|
|
const h = headersForUrl(url, TOKEN);
|
|
assert.equal("Authorization" in h, false, "Authorization header must not be present");
|
|
assert.equal(h["User-Agent"], "issue-images-mcp");
|
|
});
|
|
}
|
|
|
|
check("no token configured => no Authorization even for allowlisted host", () => {
|
|
const h = headersForUrl("https://api.github.com/x", undefined);
|
|
assert.equal("Authorization" in h, false);
|
|
});
|
|
|
|
check("extra headers are preserved", () => {
|
|
const h = headersForUrl("https://api.github.com/x", TOKEN, { "Accept": "application/vnd.github+json" });
|
|
assert.equal(h["Accept"], "application/vnd.github+json");
|
|
assert.equal(h["Authorization"], `Bearer ${TOKEN}`);
|
|
});
|
|
|
|
console.log("");
|
|
if (failures > 0) {
|
|
console.error(`${failures} test(s) failed.`);
|
|
process.exit(1);
|
|
}
|
|
console.log("All token-allowlist regression tests passed.");
|