Simplify Tinfoil proxy passthrough and remove legacy routing#166
Merged
AnthonyRonning merged 2 commits intomasterfrom Apr 9, 2026
Merged
Simplify Tinfoil proxy passthrough and remove legacy routing#166AnthonyRonning merged 2 commits intomasterfrom
AnthonyRonning merged 2 commits intomasterfrom
Conversation
WalkthroughThe pull request removes document upload/status handling, simplifies proxy routing by eliminating model-specific routing tables and replacing multi-route selection with a single completion-proxy approach, refactors tinfoil-proxy from typed endpoint handlers to a generic request passthrough, updates Go dependencies and model name canonicalization (llama-3.3-70b to llama3-3-70b), and updates PCR history records. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Axum Server
participant Proxy Router
participant Tinfoil Proxy
participant Upstream Service
Client->>Axum Server: POST /v1/chat/completions
Axum Server->>Proxy Router: get_completion_proxy()
Proxy Router-->>Axum Server: ProxyConfig (tinfoil or default)
Axum Server->>Tinfoil Proxy: Forward request (method/path/query/body)
Tinfoil Proxy->>Upstream Service: Forward with Authorization header
Upstream Service-->>Tinfoil Proxy: Response (status/body/headers)
Tinfoil Proxy->>Tinfoil Proxy: Passthrough response & canonicalize if tinfoil
Tinfoil Proxy-->>Axum Server: Stream response body
Axum Server-->>Client: Forwarded response
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
AnthonyRonning
force-pushed
the
fix/simplify-tinfoil-proxy
branch
from
434d781 to
bb0fbdb
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
AnthonyRonning
force-pushed
the
fix/simplify-tinfoil-proxy
branch
from
bb0fbdb to
c8695c0
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tinfoil-proxy/main_test.go (1)
17-32: Header-skip coverage looks good; consider adding lowercase variants.Optional hardening: add cases like
authorization/connectionto explicitly lock in case-insensitive behavior expectations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinfoil-proxy/main_test.go` around lines 17 - 32, The test TestShouldSkipHeader should include lowercase header variants to assert shouldSkipHeader is case-insensitive; update the tests map in TestShouldSkipHeader to add entries for "authorization" and "connection" (and any other lowercase forms you want) with the same expected boolean values, ensuring the test iterates over those keys so shouldSkipHeader("authorization") and shouldSkipHeader("connection") are explicitly verified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tinfoil-proxy/main_test.go`:
- Around line 17-32: The test TestShouldSkipHeader should include lowercase
header variants to assert shouldSkipHeader is case-insensitive; update the tests
map in TestShouldSkipHeader to add entries for "authorization" and "connection"
(and any other lowercase forms you want) with the same expected boolean values,
ensuring the test iterates over those keys so shouldSkipHeader("authorization")
and shouldSkipHeader("connection") are explicitly verified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5514453e-bd15-4f3d-8c9f-075c2788cf58
⛔ Files ignored due to path filters (3)
flake.lockis excluded by!**/*.locktinfoil-proxy/dist/tinfoil-proxyis excluded by!**/dist/**tinfoil-proxy/go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
flake.nixsrc/main.rssrc/proxy_config.rssrc/tokens.rssrc/web/documents.rssrc/web/health_routes.rssrc/web/mod.rssrc/web/openai.rssrc/web/responses/handlers.rssrc/web/responses/prompts.rstinfoil-proxy/go.modtinfoil-proxy/main.gotinfoil-proxy/main_test.go
💤 Files with no reviewable changes (2)
- src/web/mod.rs
- src/web/documents.rs
✅ Files skipped from review due to trivial changes (2)
- src/web/responses/handlers.rs
- tinfoil-proxy/main.go
🚧 Files skipped from review as they are similar to previous changes (3)
- src/web/health_routes.rs
- src/web/responses/prompts.rs
- tinfoil-proxy/go.mod
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Keep the raw Tinfoil proxy compliant with hop-by-hop header stripping and bounded response start without capping live SSE streams. Move the newer Go tooling into a dedicated tinfoil-proxy flake so the root Nitro/EIF flake stays on its original setup. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
AnthonyRonning
force-pushed
the
fix/simplify-tinfoil-proxy
branch
from
c8695c0 to
279ae60
Compare
Refresh the dev and prod PCR snapshots and append the new signed measurements after the latest EIF updates. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/proxy_config.rs (2)
41-47:⚠️ Potential issue | 🟠 MajorWhitespace-only
tinfoil_basestill accepted.If the env var is set to
" ",get_completion_proxy()will prefer an invalid Tinfoil endpoint overdefault_proxy, causing hard failures. Trim before checking emptiness.
,Suggested fix
let tinfoil_proxy = tinfoil_base - .filter(|base| !base.is_empty()) - .map(|base_url| ProxyConfig { - base_url, + .map(|base| base.trim().to_string()) + .filter(|base| !base.is_empty()) + .map(|base_url| ProxyConfig { + base_url, api_key: None, provider_name: "tinfoil".to_string(), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/proxy_config.rs` around lines 41 - 47, The tinfoil_base value is currently checked with .filter(|base| !base.is_empty()), which accepts whitespace-only strings; update get_completion_proxy() so tinfoil_base is trimmed before emptiness check and before constructing ProxyConfig: compute a trimmed version (e.g. let trimmed = base.trim().to_string()) and use .filter(|base| !base.trim().is_empty()) or map from the trimmed string so tinfoil_proxy only contains valid non-whitespace base_url when creating ProxyConfig (referencing tinfoil_base, tinfoil_proxy, and get_completion_proxy).
29-38:⚠️ Potential issue | 🟠 MajorSubstring host matching still risks credential leakage.
The
contains("api.openai.com")check can match unintended hosts (e.g.,api.openai.com.evil.com) and send credentials to the wrong upstream. Parseopenai_baseas a URL and compare the host exactly.
,Suggested fix
+ let is_openai_host = url::Url::parse(&openai_base) + .ok() + .and_then(|u| u.host_str().map(|h| h.eq_ignore_ascii_case("api.openai.com"))) + .unwrap_or(false); + let default_proxy = ProxyConfig { base_url: openai_base.clone(), - api_key: if openai_base.contains("api.openai.com") { + api_key: if is_openai_host { openai_key } else { None }, - provider_name: if openai_base.contains("api.openai.com") { + provider_name: if is_openai_host { "openai".to_string() } else { "continuum".to_string() }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/proxy_config.rs` around lines 29 - 38, The current substring check using openai_base.contains("api.openai.com") can leak credentials; instead parse openai_base as a URL (e.g., using Url::parse) and compare the parsed host exactly to "api.openai.com" when deciding api_key and provider_name; if URL parsing fails or host != "api.openai.com" set api_key to None and provider_name to "continuum". Ensure you reference the openai_base variable and preserve openai_key for the api_key assignment when the host matches, and handle parse errors gracefully.
🧹 Nitpick comments (1)
tinfoil-proxy/main_test.go (1)
96-98: Useerrors.Isfor timeout assertion robustness.Direct equality is brittle if errors get wrapped;
errors.Is(err, context.DeadlineExceeded)is safer.♻️ Proposed test hardening
import ( "context" + "errors" "io" "net/http" "testing" "time" ) @@ - if err != context.DeadlineExceeded { + if !errors.Is(err, context.DeadlineExceeded) { t.Fatalf("expected context deadline exceeded, got %v", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinfoil-proxy/main_test.go` around lines 96 - 98, Replace the direct equality check against context.DeadlineExceeded with errors.Is to correctly handle wrapped errors: update the assertion in the test that currently does "if err != context.DeadlineExceeded { ... }" to "if !errors.Is(err, context.DeadlineExceeded) { ... }" and add the "errors" import if missing; this targets the timeout assertion around the test's error variable (err) so wrapped timeout errors are detected reliably.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tinfoil-proxy/main_test.go`:
- Around line 37-57: The test reveals that headerSkipSet building in copyHeaders
(tinfoil-proxy/main.go) doesn't split comma-separated Connection header tokens,
so update the code that processes the "Connection" header (the logic that
currently populates headerSkipSet) to split each Connection header value on
commas, trim spaces, normalize case, and add each token to headerSkipSet before
copying; ensure copyHeaders still skips any header whose canonicalized name is
in headerSkipSet (e.g., "X-Per-Connection") so comma-delimited tokens are
correctly honored and those nominated hop-by-hop headers are not forwarded.
---
Duplicate comments:
In `@src/proxy_config.rs`:
- Around line 41-47: The tinfoil_base value is currently checked with
.filter(|base| !base.is_empty()), which accepts whitespace-only strings; update
get_completion_proxy() so tinfoil_base is trimmed before emptiness check and
before constructing ProxyConfig: compute a trimmed version (e.g. let trimmed =
base.trim().to_string()) and use .filter(|base| !base.trim().is_empty()) or map
from the trimmed string so tinfoil_proxy only contains valid non-whitespace
base_url when creating ProxyConfig (referencing tinfoil_base, tinfoil_proxy, and
get_completion_proxy).
- Around line 29-38: The current substring check using
openai_base.contains("api.openai.com") can leak credentials; instead parse
openai_base as a URL (e.g., using Url::parse) and compare the parsed host
exactly to "api.openai.com" when deciding api_key and provider_name; if URL
parsing fails or host != "api.openai.com" set api_key to None and provider_name
to "continuum". Ensure you reference the openai_base variable and preserve
openai_key for the api_key assignment when the host matches, and handle parse
errors gracefully.
---
Nitpick comments:
In `@tinfoil-proxy/main_test.go`:
- Around line 96-98: Replace the direct equality check against
context.DeadlineExceeded with errors.Is to correctly handle wrapped errors:
update the assertion in the test that currently does "if err !=
context.DeadlineExceeded { ... }" to "if !errors.Is(err,
context.DeadlineExceeded) { ... }" and add the "errors" import if missing; this
targets the timeout assertion around the test's error variable (err) so wrapped
timeout errors are detected reliably.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3b1ec261-57fa-455b-8bc6-89c382a903bc
⛔ Files ignored due to path filters (5)
pcrDev.jsonis excluded by!pcrDev.jsonpcrProd.jsonis excluded by!pcrProd.jsontinfoil-proxy/dist/tinfoil-proxyis excluded by!**/dist/**tinfoil-proxy/flake.lockis excluded by!**/*.locktinfoil-proxy/go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
pcrDevHistory.jsonpcrProdHistory.jsonsrc/main.rssrc/proxy_config.rssrc/tokens.rssrc/web/documents.rssrc/web/health_routes.rssrc/web/mod.rssrc/web/openai.rssrc/web/responses/handlers.rssrc/web/responses/prompts.rstinfoil-proxy/flake.nixtinfoil-proxy/go.modtinfoil-proxy/main.gotinfoil-proxy/main_test.go
💤 Files with no reviewable changes (2)
- src/web/mod.rs
- src/web/documents.rs
✅ Files skipped from review due to trivial changes (5)
- pcrDevHistory.json
- src/web/responses/handlers.rs
- tinfoil-proxy/flake.nix
- pcrProdHistory.json
- src/web/openai.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- src/web/health_routes.rs
- src/tokens.rs
- src/web/responses/prompts.rs
- tinfoil-proxy/main.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Summary by CodeRabbit
Bug Fixes
Removals
Changes
Chores