ci: fix reviewer models — sonnet uses Anthropic, gpt uses GPT-5 #44
@@ -28,12 +28,33 @@ jobs:
|
||||
include:
|
||||
- name: sonnet
|
||||
token_secret: SONNET_REVIEW_TOKEN
|
||||
model: gpt-5
|
||||
provider: anthropic
|
||||
|
|
||||
llm_path: /anthropic/v1
|
||||
gpt-review-bot
commented
[MINOR] Hardcoded base_url includes an internal IP address. Consider moving base URLs to repository/environment secrets or variables to avoid exposing infrastructure details and ease environment changes. **[MINOR]** Hardcoded base_url includes an internal IP address. Consider moving base URLs to repository/environment secrets or variables to avoid exposing infrastructure details and ease environment changes.
gpt-review-bot
commented
[MINOR] Hardcoded base_url values use a plaintext HTTP internal IP (e.g., http://100.86.77.84:6655/...). Consider moving these to secrets or repository variables and preferring HTTPS to avoid exposing internal infra details and to improve transport security. **[MINOR]** Hardcoded base_url values use a plaintext HTTP internal IP (e.g., http://100.86.77.84:6655/...). Consider moving these to secrets or repository variables and preferring HTTPS to avoid exposing internal infra details and to improve transport security.
gpt-review-bot
commented
[MINOR] Original 'sonnet' matrix entry was missing provider and llm_path fields; they are now added to correctly exercise the Anthropic codepath. **[MINOR]** Original 'sonnet' matrix entry was missing provider and llm_path fields; they are now added to correctly exercise the Anthropic codepath.
|
||||
model: claude-sonnet-4-6
|
||||
[MAJOR] LLM base_url uses plain HTTP (http://100.86.77.84:6655/anthropic/v1) which will transmit the LLM_API_KEY and request contents in cleartext, risking secret and data exposure. All three base_url entries (Anthropic and OpenAI) are HTTP. **[MAJOR]** LLM base_url uses plain HTTP (http://100.86.77.84:6655/anthropic/v1) which will transmit the LLM_API_KEY and request contents in cleartext, risking secret and data exposure. All three base_url entries (Anthropic and OpenAI) are HTTP.
[NIT] Hardcoded internal IP (100.86.77.84) in the repository may disclose internal network details if the repo is public and reduces flexibility across environments. **[NIT]** Hardcoded internal IP (100.86.77.84) in the repository may disclose internal network details if the repo is public and reduces flexibility across environments.
|
||||
- name: gpt
|
||||
|
gpt-review-bot
commented
[MINOR] The model for 'sonnet' was corrected from 'gpt-5' to 'claude-sonnet-4-6' consistent with the Anthropic provider. **[MINOR]** The model for 'sonnet' was corrected from 'gpt-5' to 'claude-sonnet-4-6' consistent with the Anthropic provider.
gpt-review-bot
commented
[NIT] The matrix entry named 'gpt' now runs GPT-5 while there are other entries like 'gpt41' and 'gpt5-mini'. Consider renaming 'gpt' to 'gpt5' (or similar) for clarity and to avoid confusion about which model each job runs. **[NIT]** The matrix entry named 'gpt' now runs GPT-5 while there are other entries like 'gpt41' and 'gpt5-mini'. Consider renaming 'gpt' to 'gpt5' (or similar) for clarity and to avoid confusion about which model each job runs.
|
||||
token_secret: GPT_REVIEW_TOKEN
|
||||
provider: openai
|
||||
llm_path: /openai/v1
|
||||
model: gpt-5
|
||||
|
gpt-review-bot
commented
[MINOR] The previously missing provider and llm_path fields for 'gpt41' and subsequent jobs are added, ensuring full specification of matrix entries. **[MINOR]** The previously missing provider and llm_path fields for 'gpt41' and subsequent jobs are added, ensuring full specification of matrix entries.
|
||||
- name: gpt41
|
||||
token_secret: GPT_REVIEW_TOKEN
|
||||
|
gpt-review-bot
commented
[MINOR] Matrix entries gpt41, gpt5-mini, and gpt41-mini use token_secret SONNET_REVIEW_TOKEN despite being OpenAI-based reviewers. Consider using a distinct reviewer token (e.g., GPT_REVIEW_TOKEN or dedicated tokens) to avoid confusing identity/permission scopes. **[MINOR]** Matrix entries gpt41, gpt5-mini, and gpt41-mini use token_secret SONNET_REVIEW_TOKEN despite being OpenAI-based reviewers. Consider using a distinct reviewer token (e.g., GPT_REVIEW_TOKEN or dedicated tokens) to avoid confusing identity/permission scopes.
sonnet-review-bot
commented
[MAJOR] The matrix entry 'gpt41' sets token_secret: SONNET_REVIEW_TOKEN. This is likely incorrect — gpt-related entries should probably use GPT_REVIEW_TOKEN (or their own appropriate secret). Using the wrong token will cause that matrix job to authenticate as the wrong reviewer and may fail or operate under unexpected permissions. **[MAJOR]** The matrix entry 'gpt41' sets token_secret: SONNET_REVIEW_TOKEN. This is likely incorrect — gpt-related entries should probably use GPT_REVIEW_TOKEN (or their own appropriate secret). Using the wrong token will cause that matrix job to authenticate as the wrong reviewer and may fail or operate under unexpected permissions.
|
||||
provider: openai
|
||||
llm_path: /openai/v1
|
||||
model: gpt-4.1
|
||||
- name: gpt5-mini
|
||||
|
sonnet-review-bot
commented
[MINOR] The gpt41, gpt5-mini, and gpt41-mini matrix entries all use token_secret: SONNET_REVIEW_TOKEN rather than their own dedicated secrets or GPT_REVIEW_TOKEN. This appears intentional (sharing a token for auxiliary reviewers) but is worth confirming — if the Anthropic token is used to call an OpenAI endpoint it may fail depending on how the HAI proxy validates tokens. **[MINOR]** The gpt41, gpt5-mini, and gpt41-mini matrix entries all use token_secret: SONNET_REVIEW_TOKEN rather than their own dedicated secrets or GPT_REVIEW_TOKEN. This appears intentional (sharing a token for auxiliary reviewers) but is worth confirming — if the Anthropic token is used to call an OpenAI endpoint it may fail depending on how the HAI proxy validates tokens.
|
||||
token_secret: GPT_REVIEW_TOKEN
|
||||
|
sonnet-review-bot
commented
[MAJOR] The matrix entry 'gpt5-mini' sets token_secret: SONNET_REVIEW_TOKEN. This duplicates the same token as 'sonnet' and likely should use GPT_REVIEW_TOKEN (or a dedicated token). Confirm intended reviewer tokens per matrix entry. **[MAJOR]** The matrix entry 'gpt5-mini' sets token_secret: SONNET_REVIEW_TOKEN. This duplicates the same token as 'sonnet' and likely should use GPT_REVIEW_TOKEN (or a dedicated token). Confirm intended reviewer tokens per matrix entry.
|
||||
provider: openai
|
||||
llm_path: /openai/v1
|
||||
model: gpt-5-mini
|
||||
|
gpt-review-bot
commented
[MINOR] LLM_BASE_URL is now constructed from secret plus matrix.llm_path, improving modularity and correctness in endpoint invocation. **[MINOR]** LLM_BASE_URL is now constructed from secret plus matrix.llm_path, improving modularity and correctness in endpoint invocation.
|
||||
- name: gpt41-mini
|
||||
|
sonnet-review-bot
commented
[MINOR] In the 'Run ${{ matrix.name }} review' step environment, the LLM_BASE_URL was corrected from referring to a secret to referencing the matrix.base_url, ensuring each job uses the appropriate base URL. **[MINOR]** In the 'Run ${{ matrix.name }} review' step environment, the LLM_BASE_URL was corrected from referring to a secret to referencing the matrix.base_url, ensuring each job uses the appropriate base URL.
sonnet-review-bot
commented
[MINOR] Added LLM_PROVIDER environment variable to the review step to reflect the provider set in the matrix, ensuring consistency and correct provider usage at runtime. **[MINOR]** Added LLM_PROVIDER environment variable to the review step to reflect the provider set in the matrix, ensuring consistency and correct provider usage at runtime.
|
||||
token_secret: GPT_REVIEW_TOKEN
|
||||
|
sonnet-review-bot
commented
[MAJOR] The matrix entry 'gpt41-mini' sets token_secret: SONNET_REVIEW_TOKEN. As above, this seems inconsistent with the 'gpt' naming and should likely reference GPT_REVIEW_TOKEN or another correct secret. **[MAJOR]** The matrix entry 'gpt41-mini' sets token_secret: SONNET_REVIEW_TOKEN. As above, this seems inconsistent with the 'gpt' naming and should likely reference GPT_REVIEW_TOKEN or another correct secret.
|
||||
provider: openai
|
||||
llm_path: /openai/v1
|
||||
|
gpt-review-bot
commented
[MINOR] LLM_PROVIDER environment variable added to the run step to allow correct provider logic downstream. **[MINOR]** LLM_PROVIDER environment variable added to the run step to allow correct provider logic downstream.
|
||||
model: gpt-4.1-mini
|
||||
- name: security
|
||||
token_secret: SECURITY_REVIEW_TOKEN
|
||||
|
[MINOR] LLM_BASE_URL now derives from the workflow matrix instead of a secret, increasing the risk that a PR modifying this workflow could redirect requests (and exfiltrate secrets) to an attacker-controlled host. Keeping the endpoint in a secret or protected environment reduces this risk. **[MINOR]** LLM_BASE_URL now derives from the workflow matrix instead of a secret, increasing the risk that a PR modifying this workflow could redirect requests (and exfiltrate secrets) to an attacker-controlled host. Keeping the endpoint in a secret or protected environment reduces this risk.
|
||||
provider: openai
|
||||
llm_path: /openai/v1
|
||||
model: gpt-5
|
||||
|
gpt-review-bot
commented
[NIT] LLM_BASE_URL now comes from matrix.base_url; if a secrets-based LLM_BASE_URL was previously used, ensure any obsolete secret or documentation references are cleaned up to prevent confusion. **[NIT]** LLM_BASE_URL now comes from matrix.base_url; if a secrets-based LLM_BASE_URL was previously used, ensure any obsolete secret or documentation references are cleaned up to prevent confusion.
|
||||
system_prompt_file: SECURITY_REVIEW.md
|
||||
steps:
|
||||
@@ -49,9 +70,10 @@ jobs:
|
||||
PR_NUMBER: ${{ github.event.pull_request.number }}
|
||||
REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }}
|
||||
REVIEWER_NAME: ${{ matrix.name }}
|
||||
LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }}
|
||||
LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }}${{ matrix.llm_path }}
|
||||
|
gpt-review-bot
commented
[MINOR] LLM_BASE_URL is constructed by concatenating the secret and matrix.llm_path: ${{ secrets.LLM_BASE_URL }}${{ matrix.llm_path }}. This works but is fragile: it assumes the secret does not already include the path or a trailing slash and that a single secret can be used for multiple providers. Consider storing provider-specific base URL secrets (e.g. LLM_BASE_URL_ANTHROPIC, LLM_BASE_URL_OPENAI) or normalizing/trimming slashes in the secret to avoid accidental double slashes or broken endpoints. **[MINOR]** LLM_BASE_URL is constructed by concatenating the secret and matrix.llm_path: ${{ secrets.LLM_BASE_URL }}${{ matrix.llm_path }}. This works but is fragile: it assumes the secret does not already include the path or a trailing slash and that a single secret can be used for multiple providers. Consider storing provider-specific base URL secrets (e.g. LLM_BASE_URL_ANTHROPIC, LLM_BASE_URL_OPENAI) or normalizing/trimming slashes in the secret to avoid accidental double slashes or broken endpoints.
|
||||
LLM_API_KEY: ${{ secrets.LLM_API_KEY }}
|
||||
|
[MAJOR] LLM_BASE_URL is now sourced from the PR-controlled matrix (matrix.base_url) instead of a repository secret, enabling an attacker to modify the workflow in a PR to redirect outbound requests (including secrets like LLM_API_KEY and REVIEWER_TOKEN) to an arbitrary endpoint. This is a classic supply-chain/secret exfiltration vector for CI workflows that run on pull_request events with secrets available. **[MAJOR]** LLM_BASE_URL is now sourced from the PR-controlled matrix (matrix.base_url) instead of a repository secret, enabling an attacker to modify the workflow in a PR to redirect outbound requests (including secrets like LLM_API_KEY and REVIEWER_TOKEN) to an arbitrary endpoint. This is a classic supply-chain/secret exfiltration vector for CI workflows that run on pull_request events with secrets available.
|
||||
LLM_MODEL: ${{ matrix.model }}
|
||||
LLM_PROVIDER: ${{ matrix.provider }}
|
||||
|
sonnet-review-bot
commented
[NIT] SYSTEM_PROMPT_FILE is set from matrix.system_prompt_file; for entries that don't define it the variable will be empty — that's probably fine, but consider documenting that behavior in the PR body or adding an explicit null/empty entry for clarity. **[NIT]** SYSTEM_PROMPT_FILE is set from matrix.system_prompt_file; for entries that don't define it the variable will be empty — that's probably fine, but consider documenting that behavior in the PR body or adding an explicit null/empty entry for clarity.
|
||||
CONVENTIONS_FILE: "CONVENTIONS.md"
|
||||
PATTERNS_REPO: "rodin/go-patterns"
|
||||
PATTERNS_FILES: "README.md,patterns/"
|
||||
|
||||
[MINOR] The configured LLM endpoints use HTTP (plaintext) rather than HTTPS (e.g., base_url: http://100.86.77.84:6655/...), risking interception of API keys and PR content in transit, even on internal networks.
[MINOR] The workflow hardcodes internal IP addresses and ports in a repo file, which can disclose internal network topology if the repository is public. Prefer using secrets or environment variables to avoid exposing infrastructure details.
[MINOR] You've added provider: anthropic and base_url for the 'sonnet' entry which is good. Make sure the review-bot supports Anthropic-style auth/headers when LLM_PROVIDER=anthropic and that a single LLM_API_KEY secret is valid for both Anthropic and OpenAI endpoints (or provide separate secrets if needed).