push_to_review_firm.yml: strip refreshToken to stop silent CONFIG_JSON poisoning

[michieldegezelle]

Problem

push_to_review_firm.yml (#24) documents itself as read-only on CONFIG_JSON — "never refreshes tokens and never writes the secret back." That's true of the workflow script, but not of the silverfin-cli it shells out to: axiosFactory.js's response interceptor transparently refreshes the access token on any 401, for any command, and persists the rotated pair only to the local ~/.silverfin/config.json on the runner (discarded when the job ends).

Since this workflow is dispatched frequently and at unpredictable times (a Jira button, not tied to PR activity) and is chatty against one firm (per-template updates + a full add-shared-part --all sweep), a run whose access token happens to be near/at its ~2h expiry silently rotates the refresh token server-side and throws the new pair away — leaving CONFIG_JSON holding an already-consumed refresh token. The next thing to use it (check_auth.yml's explicit refresh, the secret's sole writer) gets invalid_grant.

This is what happened to firm 400583 twice in two days (be_market runs 29314984998/29318715898, 2026-07-14). Fixed manually via re-authorization, but the mechanism was still live.

Fix

Blank every firm's refreshToken in the loaded local config before any CLI command runs (same mechanism already validated for the read-only test-runner in be_market's run_tests_TEST.yml). Effect:

• Valid token → nothing changes, push succeeds as before.
• Stale token → the CLI's 401 handler sends an empty refresh token, Silverfin rejects it cleanly, the job fails with a legible error instead of silently rotating anything. CONFIG_JSON's real refresh token is never touched.

Trades "silently corrupts a secret shared by every other consumer" for "this one push occasionally needs a retry."

Validation

Live-tested against be_market (throwaway PR #3028, closed after validation; firm 1355):

• Happy path (valid token): pushed clean, PR comment correct.
• Forced 401 (deliberately invalid access token, test-only commit never merged): failed cleanly (400 Bad Request / "Error refreshing credentials"), PR comment showed the failure legibly.
• No-poison check: immediately after the forced failure, triggered a real check_auth.yml refresh — all 8 firms in be_market's CONFIG_JSON, including 1355, refreshed successfully. The forced-401 run never touched the real secret.

Not in scope here (tracked separately)

• Porting the same strip into test-templates (run_tests.yml) — same theoretical exposure, not yet exercised hard enough to manifest.
• A be_market-local scheduled refresh wrapper to shrink the staleness window further (optional reliability improvement, not a correctness requirement — this fix is safe without it).
• The unrelated "Find open PRs" branch-matching failure hit in the same incident window.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1e37812c-cd5b-417e-9e14-309b8750f556

📥 Commits

Reviewing files that changed from the base of the PR and between 1b05cbf and 53489f2.

📒 Files selected for processing (1)

• .github/workflows/push_to_review_firm.yml

---

Walkthrough

The review workflow now sanitizes the generated Silverfin configuration by clearing refreshToken values for numeric firm entries before replacing the original configuration file.

Changes

Configuration token sanitization

Layer / File(s)	Summary
Strip firm refresh tokens .github/workflows/push_to_review_firm.yml	Uses jq to clear numeric-firm refreshToken values in a temporary configuration, then replaces config.json with the stripped version.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

• silverfin/bso_github_actions#24: Updates token and configuration processing in the same review-firm workflow.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main change to push_to_review_firm.yml.
Description check	✅ Passed	The description is detailed and covers the problem, fix, validation, and scope well enough to satisfy the template's intent.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ci-auth-strip-refresh-token-test

---

_{Comment @coderabbitai help to get the list of available commands.}