Skip to content

Add partner staging authorization script and guide#32

Merged
michieldegezelle merged 2 commits into
mainfrom
docs/authorize-partner-secret-guide
Jul 24, 2026
Merged

Add partner staging authorization script and guide#32
michieldegezelle merged 2 commits into
mainfrom
docs/authorize-partner-secret-guide

Conversation

@michieldegezelle

Copy link
Copy Markdown
Collaborator

Summary

  • Adds scripts/authorize-partner-secret.sh (previously local-only) — authorizes a partner api_key against staging via a throwaway HOME, then pushes the resulting config.json to that partner's PARTNER_CONFIG_JSON_<partner_id> GitHub secret.
  • Documents usage, prerequisites, and re-run conditions in the README, next to the run_sampler.yml docs that consume the secret.

Test plan

  • Ran ./scripts/authorize-partner-secret.sh <partner_id> nl against staging and confirmed PARTNER_CONFIG_JSON_<partner_id> was updated on the target repo
  • Confirmed run_sampler.yml picks up the resulting secret on its next run for that partner

🤖 Generated with Claude Code

Adds scripts/authorize-partner-secret.sh (previously a local-only script)
so any team can authorize a new partner api_key against staging and push
it as that partner's PARTNER_CONFIG_JSON_<partner_id> secret, and
documents usage/prerequisites/re-run conditions in the README next to
the run_sampler.yml docs that consume the secret.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

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: 34d2ccf4-e316-4cbc-8994-99b065907489

📥 Commits

Reviewing files that changed from the base of the PR and between 682ba86 and 925e838.

📒 Files selected for processing (1)
  • README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • README.md

Walkthrough

Adds a Bash utility to authorize a staging partner and upload its generated configuration as a GitHub Actions secret. The README documents the script’s usage, prerequisites, workflow, and rerun conditions.

Changes

Partner staging authorization

Layer / File(s) Summary
Authorization and secret upload
scripts/authorize-partner-secret.sh
Adds argument parsing, hidden API-key input, temporary HOME isolation, Silverfin authorization, market repository resolution, and GitHub secret upload.
Usage and staging guidance
README.md
Adds table-of-contents and detailed documentation for prerequisites, commands, operational behavior, and rerun conditions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning It includes a summary and test plan, but misses the template's Description, Fixes #, Type of change, and Checklist sections. Add the missing template sections: Description, Fixes #, Type of change, and Checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: a new partner staging authorization script plus documentation.
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 docs/authorize-partner-secret-guide

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Around line 288-293: Clarify the credential ownership contract in the README
section for scripts/authorize-partner-secret.sh: state that this script only
bootstraps the initial PARTNER_CONFIG_JSON_<partner_id> secret, while
run_sampler.yml is the sole writer for all subsequent token-rotation updates.

In `@scripts/authorize-partner-secret.sh`:
- Around line 28-30: In scripts/authorize-partner-secret.sh, validate PARTNER_ID
immediately after its assignment and before any API-key prompt or authorization
command, rejecting values that are not numeric with a clear usage/error message.
Preserve the existing MARKET and HOST argument handling and continue only for
valid partner IDs.
- Line 46: Align the secret-handling behavior across
scripts/authorize-partner-secret.sh:46 and README.md:314-318: either change the
authorize-partner invocation to use a non-argv secret mechanism, or update the
README to accurately state that API_KEY is passed via -k and may appear in
process listings; ensure both sites consistently document and implement the
chosen behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b39e5682-c72c-41d8-aa46-a2af6dcc9351

📥 Commits

Reviewing files that changed from the base of the PR and between 53e0ebd and 682ba86.

📒 Files selected for processing (2)
  • README.md
  • scripts/authorize-partner-secret.sh

Comment thread README.md
Comment on lines +288 to +293
* The partner must already be authorized with the Silverfin CLI (`silverfin authorize-partner`) and its `config.json` stored as the `PARTNER_CONFIG_JSON_<partner>` secret.

#### Authorizing a partner for staging (`scripts/authorize-partner-secret.sh`)

_Description:_
One-off setup script that authorizes a partner `api_key` against staging and stores the result as that partner's `PARTNER_CONFIG_JSON_<partner_id>` GitHub secret — the secret [`run_sampler.yml`](#run-liquid-sampler-run_sampleryml) reads from. Run this once per partner, and again any time a partner's staging token is lost for good (see _When you need to re-run this_ below) — day-to-day token rotation during sampler runs is handled automatically by the workflow itself and does **not** need this script.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Clarify the bootstrap exception to the “sole writer” contract.

The README says the workflow is the sole writer, while this section says the script uploads the initial secret. State explicitly that this script bootstraps credentials and run_sampler.yml owns subsequent token-rotation writes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 288 - 293, Clarify the credential ownership contract
in the README section for scripts/authorize-partner-secret.sh: state that this
script only bootstraps the initial PARTNER_CONFIG_JSON_<partner_id> secret,
while run_sampler.yml is the sole writer for all subsequent token-rotation
updates.

Comment on lines +28 to +30
PARTNER_ID="${1:?Usage: $0 <partner_id> <market> [host]}"
MARKET="${2:?Usage: $0 <partner_id> <market> [host]}"
HOST="${3:-https://bso-staging-beta.staging.getsilverfin.com}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reject non-numeric partner IDs before authorization.

The README defines partner_id as numeric, but arbitrary values currently flow into the Silverfin command and GitHub secret name. Validate the input before prompting for the API key.

Proposed fix
 PARTNER_ID="${1:?Usage: $0 <partner_id> <market> [host]}"
 MARKET="${2:?Usage: $0 <partner_id> <market> [host]}"
 HOST="${3:-https://bso-staging-beta.staging.getsilverfin.com}"
+
+if [[ ! "$PARTNER_ID" =~ ^[0-9]+$ ]]; then
+  printf '%s\n' "partner_id must be numeric" >&2
+  exit 2
+fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
PARTNER_ID="${1:?Usage: $0 <partner_id> <market> [host]}"
MARKET="${2:?Usage: $0 <partner_id> <market> [host]}"
HOST="${3:-https://bso-staging-beta.staging.getsilverfin.com}"
PARTNER_ID="${1:?Usage: $0 <partner_id> <market> [host]}"
MARKET="${2:?Usage: $0 <partner_id> <market> [host]}"
HOST="${3:-https://bso-staging-beta.staging.getsilverfin.com}"
if [[ ! "$PARTNER_ID" =~ ^[0-9]+$ ]]; then
printf '%s\n' "partner_id must be numeric" >&2
exit 2
fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/authorize-partner-secret.sh` around lines 28 - 30, In
scripts/authorize-partner-secret.sh, validate PARTNER_ID immediately after its
assignment and before any API-key prompt or authorization command, rejecting
values that are not numeric with a clear usage/error message. Preserve the
existing MARKET and HOST argument handling and continue only for valid partner
IDs.

trap 'rm -rf "$TMP_HOME"' EXIT

HOME="$TMP_HOME" silverfin config --set-host "$HOST"
HOME="$TMP_HOME" silverfin authorize-partner -i "$PARTNER_ID" -k "$API_KEY" -n "partner-${PARTNER_ID}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Inspecting relevant files...\n'
git ls-files scripts/authorize-partner-secret.sh README.md
printf '\n--- scripts/authorize-partner-secret.sh ---\n'
cat -n scripts/authorize-partner-secret.sh | sed -n '1,120p'
printf '\n--- README.md excerpt ---\n'
cat -n README.md | sed -n '300,330p'

printf '\n--- Searching for authorize-partner usage and supported input paths ---\n'
rg -n --hidden --no-messages 'authorize-partner|api_key|hidden input|stdin|env|secret' .

Repository: silverfin/bso_github_actions

Length of output: 24668


🌐 Web query:

Silverfin CLI authorize-partner API key stdin environment variable -k documentation

💡 Result:

The Silverfin CLI does not have a native "authorize-partner" command that accepts an API key via stdin or environment variables as a single automated step [1][2]. Authorization for the Silverfin CLI is handled through the silverfin authorize command [1][2]. This command initiates an interactive OAuth2 flow, which requires a user to: 1. Run silverfin authorize in their terminal [1][2]. 2. Log in through a browser window that opens automatically [1][2]. 3. Copy an authorization code provided by the browser and paste it into the terminal when prompted [2]. Regarding configuration and environment variables: - API Credentials: The client_id and client_secret (obtained when registering an API application) are configured via environment variables SF_API_CLIENT_ID and SF_API_SECRET [1][3]. These are typically set in shell configuration files (e.g., ~/.bashrc or ~/.zshrc) [1][3]. - Automation: For CI/CD environments (such as GitHub Actions), automation is generally achieved by managing a configuration file (often referred to as CONFIG_JSON in examples) that stores refreshed tokens, rather than by programmatically "authorizing" a partner key via stdin [4]. If you are looking to automate authentication in a non-interactive environment, you should reference your repository's configuration management workflow, which typically involves maintaining a persistent token file [4]. There is no documented support for piping a partner API key directly into an "authorize-partner" command [1][2].

Citations:


Avoid claiming the API key stays out of argv.

  • scripts/authorize-partner-secret.sh#L46 still passes API_KEY to silverfin authorize-partner -k ..., so it can be exposed via process listings.
  • README.md#L318 should match that behavior, or the script should be changed to use a non-argv secret path.
📍 Affects 2 files
  • scripts/authorize-partner-secret.sh#L46-L46 (this comment)
  • README.md#L314-L318
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/authorize-partner-secret.sh` at line 46, Align the secret-handling
behavior across scripts/authorize-partner-secret.sh:46 and README.md:314-318:
either change the authorize-partner invocation to use a non-argv secret
mechanism, or update the README to accurately state that API_KEY is passed via
-k and may appear in process listings; ensure both sites consistently document
and implement the chosen behavior.

Everyone defaults to live.getsilverfin.com; add the exact steps to
reach the staging beta host, get past its basic-auth gate, log into
the specific partner env, and pull a fresh api_key via "Configuration
partners" > "Refresh API token" before running the script.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@michieldegezelle
michieldegezelle merged commit a31208b into main Jul 24, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant