fix(ci): route Strix through Vertex AI org secrets - #288
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR extends the Strix security scanning workflow to support Vertex AI as a provider route alongside direct OpenAI, implementing model-driven provider gating, conditional credential handling, Vertex credential preparation in CI, updated timeout/fallback behavior in the gate script, and comprehensive test/documentation updates reflecting the new provider contract. ChangesVertex AI Provider Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR governance metadata gate is not ready for
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
PR governance metadata gate is not ready for
|
1 similar comment
|
PR governance metadata gate is not ready for
|
|
PR governance metadata gate is not ready for
|
45fbaf6 to
fc1e19e
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Strix Vertex routing is implemented and locally verified on head Evidence:
Root cause confirmed:
I did not reset gh secret set GCP_SA_KEY --org Seongho-Bae --visibility all < gcp-sa-key.json
# or selected access if preferred:
gh secret set GCP_SA_KEY --org Seongho-Bae --repos naruon < gcp-sa-key.jsonAfter that, rerun Strix on this PR. The workflow intentionally does not use GitHub Models or |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.github/workflows/strix.yml:
- Around line 81-110: The workflow currently adds a Vertex/Gemini branch and
defaults STRIX_MODEL to a Vertex value; remove the entire
vertex_ai/gemini-3.1-pro-preview-customtools case block (including handling of
STRIX_VERTEX_CREDENTIALS, GCP_SA_KEY, trimmed_vertex_credentials, and the
'provider_mode=vertex_ai' / 'enabled' exports) and ensure STRIX_MODEL's default
is an OpenAI GPT-5.4+ identifier (e.g., remove
"vertex_ai/gemini-3.1-pro-preview-customtools" from STRIX_MODEL default),
leaving only the OpenAI branch that validates STRIX_OPENAI_API_KEY, writes
provider_mode=openai_direct, and requires a non-empty trimmed_openai_key; also
remove any other vertex/GCP-related envs or exports referenced elsewhere (e.g.,
STRIX_VERTEX_CREDENTIALS, GCP_SA_KEY) so the workflow enforces OpenAI-only
GPT-5.4+ usage.
In `@scripts/ci/strix_quick_gate.sh`:
- Around line 171-189: The checks use raw STRIX_LLM so models that normalize to
Vertex or a Vertex fallback still require an OpenAI key; call normalize_model on
STRIX_LLM (and on the fallback variable used in this script, e.g.
STRIX_LLM_FALLBACK or the script's fallback name) and use the normalized values
with is_vertex_model when deciding whether an API key is required; update the
conditional guards around LLM_API_KEY_FILE, the resolve_trusted_input_file call,
and the empty-key check to treat the LLM as Vertex if either the normalized
primary or normalized fallback is a Vertex model so Vertex-mode runs don’t
wrongly require an OpenAI key.
- Around line 1913-1914: The code unconditionally copies STRIX_CHILD_LLM_API_KEY
into child_env["LLM_API_KEY"], which leaks the global OpenAI key into Vertex
attempts; change the assignment so it only sets child_env["LLM_API_KEY"] for
non-Vertex providers — e.g. check the child run's provider indicator (use
whatever variable you already use to pick provider, such as
os.environ.get("STRIX_CHILD_PROVIDER") or child_env.get("PROVIDER")) and only
set child_env["LLM_API_KEY"] if that provider is not "vertex"
(case-insensitive); otherwise skip the assignment to preserve provider
isolation.
In `@scripts/ci/test_strix_quick_gate.sh`:
- Around line 76-96: The tests were changed to accept Vertex/Gemini and make
Vertex the default, which violates the repo policy; revert and enforce
OpenAI-only expectations by removing or replacing assertions that reference
provider_mode=vertex_ai, vertex_ai/gemini-3.1-pro-preview-customtools,
GCP_SA_KEY, and GOOGLE_APPLICATION_CREDENTIALS, and instead assert
provider_mode=openai_direct, STRIX_LLM_DEFAULT_PROVIDER selecting OpenAI, and
that STRIX_OPENAI_API_KEY is required (keep the exact assertion
'STRIX_OPENAI_API_KEY is required for Strix OpenAI Platform scans'); also ensure
tests assert no generic secrets.LLM_API_KEY exposure and that LLM_API_KEY_*
variables use secrets.STRIX_OPENAI_API_KEY as in the existing
'LLM_API_KEY_SECRET'/'LLM_API_KEY' assertions. Make these changes wherever
similar Vertex-related assertions appear (the other referenced blocks) so the
suite only blesses workflows that supply explicit STRIX_OPENAI_API_KEY + OpenAI
GPT-5.4-or-newer usage.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 913a1039-1031-40d9-807b-582936ce41dc
📒 Files selected for processing (3)
.github/workflows/strix.ymlscripts/ci/strix_quick_gate.shscripts/ci/test_strix_quick_gate.sh
|
@coderabbitai review I addressed the provider-isolation findings that are valid for the current Vertex decision:
The OpenAI-only rollback suggestions are intentionally not applied because the current project instruction is to use organization secrets with |
|
✏️ Learnings added
✅ Actions performedReview triggered.
|
|
@coderabbitai review Updated the Strix provider governance docs on the current head The provider-isolation finding was already fixed in
Branch Strix evidence is running as workflow_dispatch run |
|
Kicking off a fresh review of the latest changes now. ✏️ Learnings added
✅ Actions performedReview triggered.
|
|
Actionable comments posted: 0 |
|
@coderabbitai review Updated current head
Changes now added:
Local evidence passed again: |
|
Kicking off a fresh review now. ✏️ Learnings added
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/strix.yml (1)
46-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake manual PR-scope dispatch require and use
pr_base_sha.
workflow_dispatchreruns currently materialize the trusted workspace fromgithub.sha, and they switch into PR-scoped mode wheneverpr_numberis non-empty. That means a manual run can enter PR-scope with an emptyPR_BASE_SHA, and it will execute the trusted scripts/config from the wrong base revision.scripts/ci/strix_quick_gate.shonly treats dispatch as PR-context whenPR_NUMBER,PR_BASE_SHA, andPR_HEAD_SHAare all present, so the workflow should use that same three-field predicate and feedpr_base_shaintoTRUSTED_WORKSPACE_SHA.Suggested patch
- name: Materialize trusted workspace env: GH_TOKEN: ${{ github.token }} REPOSITORY: ${{ github.repository }} - TRUSTED_WORKSPACE_SHA: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.base.sha || github.sha }} + TRUSTED_WORKSPACE_SHA: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.base.sha || (github.event.inputs.pr_number != '' && github.event.inputs.pr_base_sha || github.sha) }} run: | set -euo pipefail @@ - name: Fetch pull request head for trusted scan - if: github.event_name == 'pull_request_target' || github.event.inputs.pr_number != '' + if: github.event_name == 'pull_request_target' || (github.event.inputs.pr_number != '' && github.event.inputs.pr_base_sha != '' && github.event.inputs.pr_head_sha != '') env: PR_NUMBER: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.number || github.event.inputs.pr_number }} + PR_BASE_SHA: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.base.sha || github.event.inputs.pr_base_sha }} PR_HEAD_SHA: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.event.inputs.pr_head_sha }} run: | - if [ -z "$PR_NUMBER" ] || [ -z "$PR_HEAD_SHA" ]; then - echo "::error::PR number and head SHA are required for trusted PR-scope Strix evidence." + if [ -z "$PR_NUMBER" ] || [ -z "$PR_BASE_SHA" ] || [ -z "$PR_HEAD_SHA" ]; then + echo "::error::PR number, base SHA, and head SHA are required for trusted PR-scope Strix evidence." exit 1 fi @@ - STRIX_PROCESS_TIMEOUT_SECONDS: ${{ (github.event_name == 'pull_request_target' || github.event.inputs.pr_number != '') && '1200' || '2400' }} + STRIX_PROCESS_TIMEOUT_SECONDS: ${{ (github.event_name == 'pull_request_target' || (github.event.inputs.pr_number != '' && github.event.inputs.pr_base_sha != '' && github.event.inputs.pr_head_sha != '')) && '1200' || '2400' }} @@ - STRIX_DISABLE_PR_SCOPING: ${{ (github.event_name == 'pull_request_target' || github.event.inputs.pr_number != '') && '0' || '1' }} - GH_TOKEN: ${{ (github.event_name == 'pull_request_target' || github.event.inputs.pr_number != '') && github.token || '' }} + STRIX_DISABLE_PR_SCOPING: ${{ (github.event_name == 'pull_request_target' || (github.event.inputs.pr_number != '' && github.event.inputs.pr_base_sha != '' && github.event.inputs.pr_head_sha != '')) && '0' || '1' }} + GH_TOKEN: ${{ (github.event_name == 'pull_request_target' || (github.event.inputs.pr_number != '' && github.event.inputs.pr_base_sha != '' && github.event.inputs.pr_head_sha != '')) && github.token || '' }}Also applies to: 67-75, 247-255
🤖 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 @.github/workflows/strix.yml around lines 46 - 50, The TRUSTED_WORKSPACE_SHA assignment currently falls back to github.sha for manual dispatches even when PR-scoped inputs exist; update the env assignment for TRUSTED_WORKSPACE_SHA to use the PR base SHA only when the workflow is running in PR scope and all three PR inputs are present (PR_NUMBER, PR_BASE_SHA, PR_HEAD_SHA) — i.e. change the conditional that sets TRUSTED_WORKSPACE_SHA to require github.event_name == 'pull_request_target' (or presence of PR_NUMBER) AND non-empty PR_BASE_SHA and PR_HEAD_SHA, otherwise use github.sha; apply the same fix to the other occurrences of TRUSTED_WORKSPACE_SHA in the workflow (the blocks around the other two ranges) and ensure the variables PR_NUMBER, PR_BASE_SHA, PR_HEAD_SHA are referenced consistently and match scripts/ci/strix_quick_gate.sh expectations.
🧹 Nitpick comments (1)
scripts/ci/test_strix_quick_gate.sh (1)
71-73: ⚡ Quick winAlso pin the manual SHA inputs in this self-test.
This only locks down
pr_numberplus the error string, so a workflow regression that drops or renamesbase_sha/head_shawould still pass. Add assertions for both input keys and theirgithub.event.inputs.*reads.🔍 Suggested hardening
assert_file_contains "$workflow_file" "pr_number:" "strix workflow accepts manual PR-scope evidence inputs" + assert_file_contains "$workflow_file" "base_sha:" "strix workflow accepts manual PR base SHA input" + assert_file_contains "$workflow_file" "head_sha:" "strix workflow accepts manual PR head SHA input" assert_file_contains "$workflow_file" "github.event.inputs.pr_number" "strix workflow can run PR-scoped workflow_dispatch evidence" + assert_file_contains "$workflow_file" "github.event.inputs.base_sha" "strix workflow reads manual PR base SHA input" + assert_file_contains "$workflow_file" "github.event.inputs.head_sha" "strix workflow reads manual PR head SHA input" assert_file_contains "$workflow_file" "PR number and head SHA are required for trusted PR-scope Strix evidence" "strix workflow fails closed when manual PR-scope metadata is incomplete"🤖 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/ci/test_strix_quick_gate.sh` around lines 71 - 73, Add assertions to pin the manual SHA inputs in the self-test by extending the existing assert_file_contains checks that use $workflow_file and assert_file_contains; specifically add assertions for the input keys "base_sha" and "head_sha" and their runtime reads "github.event.inputs.base_sha" and "github.event.inputs.head_sha" (mirroring the existing checks for "pr_number" and "github.event.inputs.pr_number") so a regression that drops/renames those inputs will fail the test.
🤖 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 `@scripts/ci/test_strix_quick_gate.sh`:
- Around line 122-123: Remove the obsolete preview model string
"vertex_ai/gemini-3.1-pro-preview-customtools" from the allow-list in
scripts/ci/test_strix_quick_gate.sh and also delete the corresponding positive
case that checks for that exact model elsewhere in the script; ensure the guard
now only accepts "vertex_ai/gemini-2.5-flash" (and any existing approved
entries) so the Strix Security Scan uses STRIX_LLM with GCP_SA_KEY and the
supported Vertex model.
---
Outside diff comments:
In @.github/workflows/strix.yml:
- Around line 46-50: The TRUSTED_WORKSPACE_SHA assignment currently falls back
to github.sha for manual dispatches even when PR-scoped inputs exist; update the
env assignment for TRUSTED_WORKSPACE_SHA to use the PR base SHA only when the
workflow is running in PR scope and all three PR inputs are present (PR_NUMBER,
PR_BASE_SHA, PR_HEAD_SHA) — i.e. change the conditional that sets
TRUSTED_WORKSPACE_SHA to require github.event_name == 'pull_request_target' (or
presence of PR_NUMBER) AND non-empty PR_BASE_SHA and PR_HEAD_SHA, otherwise use
github.sha; apply the same fix to the other occurrences of TRUSTED_WORKSPACE_SHA
in the workflow (the blocks around the other two ranges) and ensure the
variables PR_NUMBER, PR_BASE_SHA, PR_HEAD_SHA are referenced consistently and
match scripts/ci/strix_quick_gate.sh expectations.
---
Nitpick comments:
In `@scripts/ci/test_strix_quick_gate.sh`:
- Around line 71-73: Add assertions to pin the manual SHA inputs in the
self-test by extending the existing assert_file_contains checks that use
$workflow_file and assert_file_contains; specifically add assertions for the
input keys "base_sha" and "head_sha" and their runtime reads
"github.event.inputs.base_sha" and "github.event.inputs.head_sha" (mirroring the
existing checks for "pr_number" and "github.event.inputs.pr_number") so a
regression that drops/renames those inputs will fail the test.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 67a0cd37-b1d1-4aed-a75c-7e4deb39e799
📒 Files selected for processing (10)
.github/workflows/strix.ymlAGENTS.mdREADME.mddocs/plans/2026-05-19-north-star-gap-closure.mddocs/plans/2026-05-27-caldav-writeback-source-registry.mddocs/plans/2026-05-27-self-sent-webdav-materialization-intent.mddocs/plans/2026-05-27-strix-openai-direct-only.mddocs/plans/2026-05-27-webdav-opaque-source-id.mdscripts/ci/strix_quick_gate.shscripts/ci/test_strix_quick_gate.sh
✅ Files skipped from review due to trivial changes (4)
- docs/plans/2026-05-27-webdav-opaque-source-id.md
- docs/plans/2026-05-27-caldav-writeback-source-registry.md
- docs/plans/2026-05-27-self-sent-webdav-materialization-intent.md
- docs/plans/2026-05-19-north-star-gap-closure.md
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/ci/strix_quick_gate.sh
|
@coderabbitai review Current head: 05022ff I addressed the latest robot review items:
Local evidence:
|
|
Kicking off a fresh review now. ✏️ Learnings added
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/ci/test_strix_quick_gate.sh`:
- Around line 3740-3743: The current checks use -n which only fails when
variables are empty; change them to assert the variables are not present in the
environment at all by testing for their existence (e.g. via printenv/env lookup)
instead of non-emptiness. Replace the `[ -n "${LLM_API_KEY_FILE:-}" ]` and
analogous `LLM_API_KEY` checks with existence checks that fail if the variable
is exported at all (for the symbols LLM_API_KEY_FILE and LLM_API_KEY in the
script), and keep the same error message and exit 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 993f44dc-fc92-49b1-b028-6b5e2b3238fb
📒 Files selected for processing (3)
.github/workflows/strix.ymlscripts/ci/strix_quick_gate.shscripts/ci/test_strix_quick_gate.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/strix.yml
- scripts/ci/strix_quick_gate.sh
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
Render
Destroyed
1 participant
Summary
vertex_ai/gemini-3.1-pro-preview-customtoolsusing orgGCP_SA_KEY.LLM_API_KEYto Vertex scans.pull_request_targetsafety: trusted workspace scripts, noactions/checkout, PR head treated as data, no GitHub Models routing.Operational setup verified
STRIX_LLMexists and repo-levelSTRIX_LLMis absent, so org secret will apply.GCP_SA_KEYand exports LiteLLM Vertex env:GOOGLE_APPLICATION_CREDENTIALS,VERTEXAI_PROJECT, andVERTEXAI_LOCATION.Verification
bash scripts/ci/test_strix_quick_gate.shpython3 - <<'PY' ... yaml.safe_load('.github/workflows/strix.yml') ... PYbash -n scripts/ci/strix_quick_gate.sh scripts/ci/test_strix_quick_gate.shgit diff --checkNotes
This PR intentionally does not use GitHub Models and does not add
models: read.Summary by CodeRabbit
New Features
Documentation
Tests
Chores