Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the api-proxy’s Google Gemini forwarding path by stripping additional SDK-injected API-key query parameter variants so the proxy-injected x-goog-api-key header isn’t ignored upstream (avoiding API_KEY_INVALID errors).
Changes:
- Extend
stripGeminiKeyParam()to deletekey,apiKey, andapi_keyquery parameters. - Add startup logging to make Gemini key configuration (present/absent) visible in container logs.
- Add new Jest test cases covering the additional query parameter variants and combinations.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/server.js | Expands Gemini auth query-param stripping and adds Gemini configuration startup logs. |
| containers/api-proxy/server.test.js | Adds test coverage for the new apiKey / api_key stripping behavior and multi-variant cases. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
|
|
||
| logRequest('info', 'server_start', { message: `GEMINI_API_KEY configured (length=${GEMINI_API_KEY.length})` }); | ||
| geminiServer.listen(10003, '0.0.0.0', () => { |
| it('should remove the apiKey= query parameter', () => { | ||
| expect(stripGeminiKeyParam('/v1/models/gemini-pro:generateContent?apiKey=placeholder')) | ||
| .toBe('/v1/models/gemini-pro:generateContent'); | ||
| }); | ||
|
|
||
| it('should remove the api_key= query parameter', () => { | ||
| expect(stripGeminiKeyParam('/v1/models/gemini-pro:generateContent?api_key=placeholder')) | ||
| .toBe('/v1/models/gemini-pro:generateContent'); | ||
| }); | ||
|
|
||
| it('should remove apiKey= while preserving other query parameters', () => { | ||
| expect(stripGeminiKeyParam('/v1/models/gemini-pro:generateContent?apiKey=placeholder&alt=json')) | ||
| .toBe('/v1/models/gemini-pro:generateContent?alt=json'); | ||
| }); | ||
|
|
||
| it('should remove api_key= while preserving other query parameters', () => { | ||
| expect(stripGeminiKeyParam('/v1/models/gemini-pro:generateContent?api_key=placeholder&alt=json')) | ||
| .toBe('/v1/models/gemini-pro:generateContent?alt=json'); | ||
| }); | ||
|
|
||
| it('should remove all auth params when multiple variants are present', () => { | ||
| expect(stripGeminiKeyParam('/v1/models/gemini-pro:generateContent?key=foo&apiKey=bar&api_key=baz&alt=json')) | ||
| .toBe('/v1/models/gemini-pro:generateContent?alt=json'); | ||
| }); |
This comment has been minimized.
This comment has been minimized.
|
@copilot address the review feedback #2182 (review) |
This comment has been minimized.
This comment has been minimized.
Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/46aa116c-47a9-4f66-8e20-13cb6f683aab Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Done in 06dfc83:
|
|
Smoke Test Results ✅ GitHub MCP: Listed 2 merged PRs Status: PASS
|
|
Smoke test report Warning The following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
🔥 Smoke Test Results
Overall: PASS PR by
|
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( PR: fix(api-proxy): strip all Gemini auth query param variants to prevent API_KEY_INVALID by Overall: PARTIAL — BYOK inference ✅, pre-step data unavailable (template vars not expanded).
|
🏗️ Build Test Suite ResultsAll repository clone attempts failed with:
Overall: 0/8 ecosystems passed — ❌ FAIL Error details: The
|
Smoke Test Results: GitHub Actions Services Connectivity
All 3 checks failed.
|
Chroot Version Comparison Results
Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot. The
|
stripGeminiKeyParamonly removed?key=but the@google/genaiSDK also appends?apiKey=and?api_key=. When those pass through with the placeholder value, Google sees the placeholder param and ignores the realx-goog-api-keyheader injected by the proxy, producingAPI_KEY_INVALID.Changes
stripGeminiKeyParam— now deletes all three auth param variants (key,apiKey,api_key) before forwardingGEMINI_API_KEY configured (length=N)on start (key present) or awarnentry (key absent), making misconfiguration visible indocker logs awf-api-proxyrather than only at first inference call