Decouple token refresh from liquid tests: read-only CONFIG_JSON in run_tests.yml

.github/workflows/run_tests.yml

@@ -1,3 +1,24 @@
+# Read-only consumer of CONFIG_JSON. This workflow never refreshes a Silverfin token and never
+# writes the CONFIG_JSON secret back: every firm's refreshToken is blanked before the CLI runs, so
+# an expired access token fails its 401 cleanly instead of silverfin-cli's auto-refresh-on-401
+# silently rotating the shared token server-side and orphaning the rotated pair on the ephemeral
+# runner (which is what poisons CONFIG_JSON and forces a manual re-authorization).
+#
+# Each caller repo refreshes its own tokens with a scheduled refresh_token.yml (*/10 cron calling
+# check_auth.yml), which is that repo's SOLE writer of CONFIG_JSON. That wrapper is a prerequisite
+# for this workflow: without it nothing refreshes the caller's tokens and every firm's access token
+# lapses within its 2h TTL. See the README's "Token refresh (refresh_token.yml in each market
+# repo)" section for the adopter checklist.
+#
+# Validated in be_market (forked inline there in be_market#3047, live since 2026-07-24); the same
+# strip mechanism is used by push_to_review_firm.yml (#29).
+#
+# Known limitation (CLI-side, tracked separately): if a firm's access token has already expired
+# when this job runs, blanking its refreshToken does not fail just that one template -
+# silverfin-cli still attempts a refresh with the blanked token, fails, and exits the process,
+# which loses the verdict for every handle batched in the same `run-test --status` call. That is a
+# CI-reliability failure (a re-runnable false negative), never a token-poisoning one, since this
+# job has no write-back path at all.
 name: run-liquid-tests
 run-name: Run liquid tests of updated reconciliations and reconciliations that use updated shared parts
 on:
@@ -11,10 +32,6 @@ on:
       - "main"
 
 jobs:
-  check-auth:
-    uses: ./.github/workflows/check_auth.yml
-    secrets: inherit
-
   check-changed-templates:
     runs-on: ubuntu-latest
     outputs:
@@ -98,7 +115,7 @@ jobs:
       # run never completed). Genuine test failures are never retried.
       MAX_TEST_ATTEMPTS: 3
     if: ${{ needs.check-changed-templates.outputs.changed_templates != '[]' }}
-    needs: [check-auth, check-changed-templates]
+    needs: [check-changed-templates]
     steps:
       - name: Checkout Repository - Fetch all history for all tags and branches
         uses: actions/checkout@v6
@@ -108,11 +125,21 @@ jobs:
         uses: actions/setup-node@v6
         with:
           node-version: 22
-      - name: Load Silverfin config file from secrets
+      - name: Load Silverfin config file from secrets (read-only, no-poison)
+        env:
+          # Passed via env, not interpolated into the script text: CONFIG_JSON is a secret blob of
+          # JSON that could contain a single quote or backtick, which would otherwise break out of
+          # a quoted `echo '...'` literal and let the secret's own content run as shell commands.
+          CONFIG_JSON: ${{ secrets.CONFIG_JSON }}
         run: |
-          mkdir -p $HOME/.silverfin/
-          touch $HOME/.silverfin/config.json
-          echo '${{ secrets.CONFIG_JSON }}' > $HOME/.silverfin/config.json
+          mkdir -p "$HOME/.silverfin/"
+          printf '%s' "${CONFIG_JSON}" > "$HOME/.silverfin/config.json"
+          # Blank every firm's refreshToken so an expired access token fails cleanly on a 401
+          # instead of silently rotating (and poisoning) the shared CONFIG_JSON secret this job
+          # never writes back to. The caller's refresh_token.yml cron is its sole writer.
+          jq 'to_entries | map(if (.key|test("^[0-9]+$")) then .value.refreshToken = "" else . end) | from_entries' \
+            "$HOME/.silverfin/config.json" > "$HOME/.silverfin/config.stripped.json"
+          mv "$HOME/.silverfin/config.stripped.json" "$HOME/.silverfin/config.json"
       - name: Add silverfin-cli package latest version
         run: |
           npm install https://github.com/silverfin/silverfin-cli.git

README.md

@@ -41,6 +41,7 @@ The document will go over all the Github Actions that currently automate a coupl
     * [Remove Code Review Label (remove_code_review_label.yml)](https://silverfin.quip.com/avPDA9TrpJ9Y#temp:C:EBf2828559422d84087b81255dc8)
   * [Authentication](https://silverfin.quip.com/avPDA9TrpJ9Y#temp:C:EBf4f743db4b6854130af8693671)
     * [Check authentication (check_auth.yml)](https://silverfin.quip.com/avPDA9TrpJ9Y#temp:C:EBf73af65c6455e4bb6a62454b15)
+    * [Token refresh (refresh_token.yml in each market repo)](#token-refresh-refresh_tokenyml-in-each-market-repo)
   * [Testing](https://silverfin.quip.com/avPDA9TrpJ9Y#temp:C:EBf13d6dbd0df1e4450b3b22691c)
     * [Check YAML files (check_tests.yml)](https://silverfin.quip.com/avPDA9TrpJ9Y#temp:C:EBf931fa1547b4b419d940464f89)
     * [Run liquid tests (run_tests.yml)](https://silverfin.quip.com/avPDA9TrpJ9Y#temp:C:EBfa4878bb5baac49cbb0c39d927)
@@ -98,9 +99,10 @@ _Trigger_:
 _Description_:
 Refreshes Silverfin API tokens to ensure authentication remains valid for subsequent operations.
 
-_Trigger_: 
+_Trigger_:
 
-* The authentication workflow is run before the `run-tests` workflow to make sure that we always have the correct authentication before communicating with the platform. 
+* Called by each market repo's own scheduled `refresh_token.yml` wrapper (a `*/10` cron). It is **not** called by `run_tests.yml` any more - see [Token refresh](#token-refresh-refresh_tokenyml-in-each-market-repo) for why the two were decoupled.
+* A `schedule:` trigger only fires from the repository that contains the workflow file, so the cron lives in each caller repo rather than here.
 
 
 _Steps:_
@@ -116,8 +118,35 @@ _Prerequisites_:
 * `SF_API_CLIENT_ID`: Silverfin API client ID
 * `SF_API_SECRET`: Silverfin API secret
 * `CONFIG_JSON`: Silverfin configuration file content
-* `REPO_ACCESS_TOKEN`: GitHub personal access token
+* `REPO_ACCESS_TOKEN`: GitHub personal access token (used for the `gh secret set` write-back; the implicit `GITHUB_TOKEN` cannot write secrets)
+
+
+#### Token refresh (`refresh_token.yml` in each market repo)
+
+_Why this exists_:
+
+`silverfin-cli` refreshes an access token transparently on **any** 401, for any command - and Silverfin rotates the refresh token server-side when it does. The rotated pair is only written to the runner's local `~/.silverfin/config.json`, never back to the secret, so `CONFIG_JSON` is left holding an already-consumed refresh token. The next explicit refresh then fails with `invalid_grant` and that firm needs a **manual re-authorization** before its CI works again. This happened in `be_market` on 2026-07-14 (firm 400583).
+
+The fix has two halves, and both are needed:
+
+1. **One scheduled writer.** A `refresh_token.yml` in the market repo, on a `*/10` cron, is the only thing that ever refreshes tokens or writes `CONFIG_JSON`.
+2. **Read-only consumers.** `run_tests.yml` and `push_to_review_firm.yml` load `CONFIG_JSON` with every firm's `refreshToken` blanked, so a stale access token fails its 401 cleanly instead of rotating anything.
+
+Live in `be_market` since 2026-07-24.
+
+_Adopter checklist_ (per market repo):
+
+* [ ] Add `.github/workflows/refresh_token.yml`: `on: schedule` (`*/10 * * * *`) + `workflow_dispatch`, calling `silverfin/bso_github_actions/.github/workflows/check_auth.yml@main` with `secrets: inherit`. Copy an existing market's file - `nl_market`, `uk_market`, `lu_market` and `be_market` all run the same one.
+* [ ] Set `permissions: contents: write` at the workflow level. This is **required, not cosmetic**: a caller-level `permissions:` block more restrictive than the callee's own top-level block makes the run fail with `startup_failure` before any job is created, with no job-level error to explain it (`gh api repos/<repo>/actions/runs/<id>/jobs` returning an empty `jobs` array is the signal). GitHub does not intersect permissions down to the caller's scope.
+* [ ] Use `secrets: inherit`, not a named list - `check_auth.yml`'s `workflow_call` trigger declares no `secrets:` block, and GitHub rejects an explicit map to a reusable workflow that has not declared matching names.
+* [ ] Provision the secrets: `SF_API_CLIENT_ID`, `SF_API_SECRET`, `CONFIG_JSON`, `REPO_ACCESS_TOKEN`, and `SLACK_CI_ALERTS_WEBHOOK_URL` for the failure alert. The Slack secret reuses the shared CI-alerts Workflow Builder trigger - one Slack workflow serves every repo, so do not create a new one and never hit *Regenerate URL* (it invalidates the URL for every repo already using it). The trigger takes a single `text` variable; the alert interpolates `github.repository` so one channel still identifies the market.
+* [ ] **Merge the cron before anything stops refreshing.** If the market starts consuming `CONFIG_JSON` read-only without a live cron, every firm's access token lapses within its 2h TTL and all liquid tests fail. The interim state (cron live *and* an older coupled refresher) is safe: `check_auth.yml`'s own job declares `concurrency: { group: refresh-tokens }`, and concurrency groups are repository-scoped, so the two serialize.
+* [ ] Verify post-merge: a `schedule`-triggered run appears green within ~10-20 minutes (`gh run list --workflow refresh_token.yml`), and `gh secret list` shows `CONFIG_JSON` with a fresh timestamp. `schedule` and `workflow_dispatch` only register from the default branch, so this cannot be tested from a PR branch.
+
+_Known limitations_ (both tracked, neither a token-safety issue):
 
+* `check_auth.yml`'s refresh loop is not fault-isolated per firm: under `bash -e`, one firm's failed refresh aborts the step and skips the write-back, so no firm's refreshed token is persisted for that tick. Transient failures self-heal on the next tick; a genuinely dead refresh token needs the manual re-authorization the Slack alert surfaces.
+* Cadence is nominal only. GitHub's `schedule` queue is best-effort - measured gaps between consecutive `*/10` runs range from ~20 to ~85 minutes, worst overnight - which is why the cadence is `*/10` against a 2h TTL rather than something looser.
 
 
 ### Testing
@@ -147,7 +176,7 @@ _Trigger_:
 
 _Steps:_
 
-* Calls `check_auth.yml` to refresh tokens
+* Loads `CONFIG_JSON` read-only, blanking every firm's `refreshToken` so the job can never rotate or poison the shared token (it does **not** refresh tokens - the caller repo's [`refresh_token.yml`](#token-refresh-refresh_tokenyml-in-each-market-repo) cron does that, and is a prerequisite)
 * Identifies changed liquid/config files
 * Determines which templates need testing
 * Runs liquid tests using silverfin-cli `run-test` command