From 7dfaff70c10cf099ccb007da7be52fbb99468db8 Mon Sep 17 00:00:00 2001 From: Andrey Talman Date: Tue, 1 Sep 2026 18:24:06 -0700 Subject: [PATCH 1/2] [CI] Fix CRCR nightly report: secret get needs --skip-redaction in containers The torch-nightly CRCR report has skipped on every run since the secret fallback landed in #54605, always with the same line: no Buildkite API token (env BUILDKITE_API_TOKEN or secret 'CRCR_BUILDKITE_API_TOKEN') -- skipping report The secret is not missing, not mis-scoped and not denied by policy. `buildkite-agent secret get` fetches it successfully and then throws it away. `clicommand/secret_get.go` fetches the secret, then builds a Job API client, then registers the value with the log redactor, and only then prints it to stdout. The Buildkite docker plugin mounts the agent binary and passes BUILDKITE_JOB_ID and BUILDKITE_AGENT_ACCESS_TOKEN, but it does not bind-mount the Job API socket or pass BUILDKITE_AGENT_JOB_API_SOCKET and BUILDKITE_AGENT_JOB_API_TOKEN. The client cannot be built, the command exits 1 with empty stdout, and the print is never reached; `|| BK_TOKEN=""` then turns that into the generic "no token" line. That the fetch itself succeeds is confirmed by the cluster secret's last_read_at: 2026-09-02T10:16:33.496Z, the same second build 86851 logged the skip. Metadata reads do not update that field, and no other crcr-report job ran near that time. This is not an agent-version problem. secret get ships in the v3.73.1 agent our AWS queues run, and the behaviour is unchanged in the current v3.138.0, which merely explains itself better and names --skip-redaction as the remedy. Upgrading the agent or the Elastic CI Stack would not have fixed it. Passing --skip-redaction is safe on this path: the value is assigned to BK_TOKEN and used only as a curl Authorization header, it is never echoed, and the script does not run under set -x. The flag must precede the key, because urfave/cli v1 stops parsing flags at the first positional argument. The alternative, bind-mounting the Job API socket into every containerized step, is a ci-infra change with a far wider blast radius for no benefit here. The stderr diagnostics are kept. Previously 2>/dev/null collapsed a missing secret, a denied policy, an unsupported subcommand, an empty value and this Job API failure into one indistinguishable line, which is why this took several build cycles to find. Only stderr is echoed; stdout is the secret and must not be logged. Behaviour is otherwise unchanged: still best-effort, still exit 0 on every failure path, still prefers BUILDKITE_API_TOKEN from the environment. Test Plan: ``` bash -n .buildkite/scripts/crcr-report.sh shellcheck -S warning .buildkite/scripts/crcr-report.sh ``` Confirmed --skip-redaction exists in the agent version the queues actually run, rather than assuming it: ``` curl -s https://proxy.golang.org/github.com/buildkite/agent/v3/@v/v3.73.1.zip -o v3.73.1.zip unzip -q v3.73.1.zip grep -n 'skip-redaction' 'github.com/buildkite/agent/v3@v3.73.1/clicommand/secret_get.go' ``` Diagnostics paths exercised against a stub buildkite-agent: exit 1 with an error message reports the agent's own message and version, exit 0 with empty stdout reports "resolved but is empty", and exit 0 with a token proceeds past the token check. In the success case the log was grepped for the stub token value: 0 matches, so the secret is not echoed. Test Result: not confirmed end to end yet. The fix needs the next torch-nightly (or a manual build with TORCH_NIGHTLY=1) to prove the report actually posts. The signal is that the "no Buildkite API token" line disappears and crcr_report.py runs. Authored with the assistance of an AI coding assistant. Signed-off-by: Andrey Talman --- .buildkite/scripts/crcr-report.sh | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/.buildkite/scripts/crcr-report.sh b/.buildkite/scripts/crcr-report.sh index a7d74fbcdab1..19333314c94f 100644 --- a/.buildkite/scripts/crcr-report.sh +++ b/.buildkite/scripts/crcr-report.sh @@ -39,10 +39,34 @@ fi # the agent exposes only its own step, so the job list comes from the REST API. TOKEN_SECRET_KEY="${CRCR_BUILDKITE_TOKEN_SECRET_KEY:-CRCR_BUILDKITE_API_TOKEN}" BK_TOKEN="${BUILDKITE_API_TOKEN:-}" -if [[ -z "${BK_TOKEN}" ]] && command -v buildkite-agent >/dev/null 2>&1; then +if [[ -z "${BK_TOKEN}" ]]; then # Not in the job environment, so read it from a Buildkite secret. The agent # redacts values fetched this way from the log. - BK_TOKEN="$(buildkite-agent secret get "${TOKEN_SECRET_KEY}" 2>/dev/null)" || BK_TOKEN="" + # + # Report why a lookup failed. Swallowing stderr made a missing secret, a + # denied policy and an unusable agent indistinguishable, all surfacing as the + # same "no token" line. Only stderr is echoed -- stdout is the secret. + if ! command -v buildkite-agent >/dev/null 2>&1; then + echo "buildkite-agent is not on PATH; cannot read secret '${TOKEN_SECRET_KEY}'" + else + secret_err="$(mktemp)" + # --skip-redaction because the step runs under the docker plugin, which + # does not expose the Job API socket to the container. Without it the + # agent fetches the secret and then refuses to print it, having no way + # to register the value with the log redactor. The value only ever + # reaches BK_TOKEN and a curl header, so it cannot land in the log. + if BK_TOKEN="$(buildkite-agent secret get --skip-redaction "${TOKEN_SECRET_KEY}" 2>"${secret_err}")"; then + if [[ -z "${BK_TOKEN}" ]]; then + echo "secret '${TOKEN_SECRET_KEY}' resolved but is empty" + fi + else + BK_TOKEN="" + echo "buildkite-agent secret get '${TOKEN_SECRET_KEY}' failed" \ + "(agent $(buildkite-agent --version 2>&1 | head -1)):" + sed 's/^/ /' "${secret_err}" + fi + rm -f "${secret_err}" + fi fi if [[ -z "${BK_TOKEN}" ]]; then echo "no Buildkite API token (env BUILDKITE_API_TOKEN or secret" \ From 2a7ee187d16958359a6014fa1a63eefbc4dca304 Mon Sep 17 00:00:00 2001 From: Andrey Talman Date: Fri, 4 Sep 2026 10:33:58 -0700 Subject: [PATCH 2/2] Drop --skip-redaction; the deployed agent cannot honour it v3.73.1's secret_get.go creates the Job API client unconditionally and only checks SkipRedaction afterwards, so under the docker plugin -- no Job API socket in the container -- it returns 'failed to create Job API client' before the flag is ever read. The ordering was only corrected in v3.107.0; every tag from v3.74.0 through v3.106.0 has the same problem. Skipping redaction would also have stopped the token being registered with the log redactor, losing that protection for no gain on this agent. Keep the diagnostics: surfacing stderr and the agent version is what makes the 'failed to create Job API client' message visible, which names the root cause instead of collapsing every failure into one 'no token' line. Signed-off-by: Andrey Talman --- .buildkite/scripts/crcr-report.sh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/.buildkite/scripts/crcr-report.sh b/.buildkite/scripts/crcr-report.sh index 19333314c94f..e19146fddbed 100644 --- a/.buildkite/scripts/crcr-report.sh +++ b/.buildkite/scripts/crcr-report.sh @@ -50,12 +50,14 @@ if [[ -z "${BK_TOKEN}" ]]; then echo "buildkite-agent is not on PATH; cannot read secret '${TOKEN_SECRET_KEY}'" else secret_err="$(mktemp)" - # --skip-redaction because the step runs under the docker plugin, which - # does not expose the Job API socket to the container. Without it the - # agent fetches the secret and then refuses to print it, having no way - # to register the value with the log redactor. The value only ever - # reaches BK_TOKEN and a curl header, so it cannot land in the log. - if BK_TOKEN="$(buildkite-agent secret get --skip-redaction "${TOKEN_SECRET_KEY}" 2>"${secret_err}")"; then + # Deliberately not passing --skip-redaction. On the deployed agent + # (v3.73.1) the flag cannot help: secret_get.go creates the Job API + # client unconditionally and only checks SkipRedaction afterwards, so + # under the docker plugin -- which does not expose the Job API socket to + # the container -- it fails before the flag is read. That ordering was + # only fixed in v3.107.0. Skipping redaction would also stop the token + # being registered with the log redactor, for no gain here. + if BK_TOKEN="$(buildkite-agent secret get "${TOKEN_SECRET_KEY}" 2>"${secret_err}")"; then if [[ -z "${BK_TOKEN}" ]]; then echo "secret '${TOKEN_SECRET_KEY}' resolved but is empty" fi