Skip to content

Commit

A brokered secret is filled in only over HTTPS. A job's request to one of the secret's hosts over plain HTTP would have carried the real value across the internet readable by anyone on the way; it now goes with the stand-in, and the job's log notes it once, as for a host the secret is not for. The secrets guide says so.

syntaqxcommitted Parent02abd97Browse files
3 files+21−80/3 viewed
+3−2
103103 Broker it, and the job gets a stand-in in its place: a token such as
104104 `g1tsec_3f9c…` (48 random characters after the prefix), made for that job
105105 and that secret alone. As the job's requests leave its sandbox, g1t puts
106−the real value in place of the stand-in on requests to the hosts the
107−secret is for. Code in the job, a compromised dependency included, can
106+the real value in place of the stand-in on HTTPS requests to the hosts
107+the secret is for; never over plain HTTP, where it would cross the
108+internet readable. Code in the job, a compromised dependency included, can
108109 read only the stand-in, which works nowhere else and stops working when
109110 the job ends.
110111
+8−0
6666 assert.deepEqual(fill("https://example.com/", new Headers({ "x-api-key": API.standIn }), secrets).filled, []);
6767 });
6868
69+test("a value is filled in only over HTTPS, even for its own host", () => {
70+ const headers = new Headers({ authorization: `Bearer ${NPM.standIn}` });
71+ const plain = fill("http://registry.npmjs.org/pkg", headers, secrets);
72+ assert.equal(plain.headers.get("authorization"), `Bearer ${NPM.standIn}`);
73+ assert.deepEqual(plain.filled, []);
74+ assert.deepEqual(plain.misdirected, [NPM]);
75+});
76+
6977 test("a stand-in the job was not given is left alone, and not reported", () => {
7078 const stranger = `g1tsec_${"c".repeat(48)}`;
7179 const out = fill("https://registry.npmjs.org/", new Headers({ authorization: `Bearer ${stranger}` }), secrets);
+10−6
8989 export function fill(url: string, headers: Headers, secrets: readonly BrokeredSecret[]): Filled {
9090 const present = new Set(standInsIn(url, headers));
9191 const carried = secrets.filter((secret) => present.has(secret.standIn));
92− const host = normalizeHost(new URL(url).host);
93− const filled = carried.filter((secret) => allows(secret.hosts, host));
94− const misdirected = carried.filter((secret) => !allows(secret.hosts, host));
92+ const parsed = new URL(url);
93+ const host = normalizeHost(parsed.host);
94+ // Only over HTTPS: a value filled into plain HTTP would cross the
95+ // internet readable by anyone on the way, its host or not. Such a
96+ // request goes with the stand-in, and is reported as misdirected.
97+ const secure = parsed.protocol === "https:";
98+ const filled = carried.filter((secret) => secure && allows(secret.hosts, host));
99+ const misdirected = carried.filter((secret) => !filled.includes(secret));
95100 if (filled.length === 0) return { url, headers, filled, misdirected };
96101
97102 const out = new Headers();
106111 });
107112 // Only the query string: a value in a path is rare, and is left as the
108113 // job wrote it.
109− const parsed = new URL(url);
110114 if (parsed.search) parsed.search = swap(parsed.search, filled, encodeURIComponent);
111115 return { url: parsed.toString(), headers: out, filled, misdirected };
112116 }
141145
142146 /**
143147 * The line a job's log gets when a brokered secret's stand-in went to a
144− * host it is not for.
148+ * host it is not for, or over plain HTTP.
145149 */
146150 export function misdirectedNote(name: string, host: string): string {
147− return `##[warning]${name} was sent to ${normalizeHost(host)}, which it is not for, so g1t did not fill it in.`;
151+ return `##[warning]${name} was sent to ${normalizeHost(host)}, which it is not for or not over HTTPS, so g1t did not fill it in.`;
148152 }
149153
150154 /** The most misdirected secrets noted in one job's log. */