Skip to content

Commit 8c163f1

Browse files
Merge pull request #24 from StepenkoAnatoli/fix/client-ref-is-public
client_ref is public: say so where a dispatcher reads it
2 parents 96da664 + aded484 commit 8c163f1

3 files changed

Lines changed: 34 additions & 2 deletions

File tree

‎.github/workflows/collect.yml‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ name: collect
2525
# `client_ref` is OPTIONAL insurance for a caller stuck on the old API version, and for
2626
# display. It is not the correlation mechanism.
2727
#
28+
# AND IT IS PUBLIC. It becomes the run name and the artifact name, both of which an
29+
# anonymous caller can read on a public repository. Everything this file does to keep the
30+
# topic out of those names is undone by a caller who puts the subject in the reference -
31+
# `layoff-plan-q3` fits the permitted shape perfectly. Treat it as an opaque job id.
32+
# The input description says so at the point of entry, which is the only place a
33+
# dispatcher is actually looking.
34+
#
2835
# THE TOPIC IS NOT IN THE RUN NAME OR THE ARTIFACT NAME
2936
#
3037
# An artifact listing and a run listing are readable by an ANONYMOUS caller on a public
@@ -65,7 +72,7 @@ on:
6572
type: choice
6673
options: [probe, quick, normal]
6774
client_ref:
68-
description: 'Optional caller reference for display and retries: [A-Za-z0-9][A-Za-z0-9._-]{0,63}. Correlation does not need it - pin X-GitHub-Api-Version and read workflow_run_id.'
75+
description: 'PUBLIC on a public repository - it becomes the run name and the artifact name. Use an opaque id (job-0417), never the subject. Optional: correlation does not need it - pin X-GitHub-Api-Version and read workflow_run_id. Shape [A-Za-z0-9][A-Za-z0-9._-]{0,63}.'
6976
required: false
7077
default: ''
7178
runner:
@@ -135,6 +142,12 @@ jobs:
135142
console.error("Refused rather than rewritten: a sanitised token means you poll for a reference the artifact does not carry.");
136143
process.exit(1);
137144
}
145+
// A WARNING, not a refusal. Whether a reference discloses a subject is a
146+
// judgement about meaning, and a regex that tried to make it would refuse
147+
// honest ids and pass `layoff-plan-q3` anyway - it fits the shape perfectly.
148+
// What this can do is make sure nobody can say they were not told, on the run
149+
// that used it, where they will see it.
150+
console.log(`::warning::client_ref "${value}" is PUBLIC: it is this run's name and the artifact's name, both readable by anyone who can read this repository. If it names the subject, that subject is disclosed.`);
138151
console.log("client_ref accepted");
139152
'
140153

‎research-kit/README.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,11 @@ configured environment can supply.
201201
would return a measurably worse corpus under the same artifact name, and the person who
202202
asked for research would have no way to tell.
203203

204+
**`client_ref` is public.** If you pass one it becomes the run name *and* the artifact
205+
name, both readable by anyone who can read the repository. Use an opaque job id
206+
(`job-0417`); never put the subject in it. `layoff-plan-q3` fits the permitted shape
207+
perfectly, which is why the workflow warns rather than pretending a regex could judge it.
208+
204209
**Privacy, stated honestly.** The topic is in neither the run name nor the artifact name,
205210
because an anonymous caller can read both listings on a public repository. But
206211
`workflow_dispatch` inputs *are* visible on the run page to anyone with read access — this
@@ -328,7 +333,7 @@ node research-kit/bin/selftest.mjs # all of it
328333
node research-kit/bin/selftest.mjs gate hook # just these files
329334
```
330335

331-
753 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means
336+
754 tests, offline, no key and no network. The runner **awaits** every test, so `ok` means
332337
the assertions settled (ADR-0021), and each test is raced against a watchdog
333338
(`RESEARCH_KIT_TEST_TIMEOUT`, default 60s).
334339

‎research-kit/test/collector-workflow.test.mjs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,20 @@ test('the step summary reports hosts and counts, never URLs', () => {
151151
'the summary prints a whole URL; a path or query string discloses what was being researched');
152152
});
153153

154+
test('the caller reference is declared PUBLIC where a dispatcher will read it', () => {
155+
// Everything this workflow does to keep the topic out of the run and artifact names is
156+
// undone by a caller who puts the subject in the reference - `layoff-plan-q3` fits the
157+
// permitted shape perfectly. No regex can judge that, so the defence is that nobody can
158+
// say they were not told, in the place they are actually looking: the dispatch form.
159+
const input = body.slice(body.indexOf('client_ref:'));
160+
const description = (input.match(/^\s*description:.*$/m) ?? [''])[0];
161+
assert.ok(/PUBLIC/.test(description),
162+
`the client_ref input does not warn that it becomes the public run and artifact name: ${description.trim()}`);
163+
164+
assert.ok(/::warning::client_ref/.test(body),
165+
'the run should also warn on the run itself, where the reference was actually used');
166+
});
167+
154168
// ---------------------------------------------------------------- correlation
155169

156170
test('the dispatch API version is pinned, and the documented one', () => {

0 commit comments

Comments
 (0)