Experimental: JWE-based credential proxy prototype#39
Conversation
Stateless proxy that decrypts JWE tokens and injects raw API credentials at the last hop before the target API. Sandboxes running agent-generated code never see raw credentials — only encrypted JWEs that are host-locked to prevent exfiltration. Uses ECDH-ES+A256KW with A256GCM on P-256 keys (go-jose/v4). Includes host locking, key rotation via JWKS, revocation deny-list, per-request tracing, runtime/secret.Do integration for memory zeroing, and manual plaintext zeroing as a cross-platform fallback. 14 integration test scenarios stress-testing credential isolation, exfiltration attempts, key rotation, concurrency, SSE streaming, token expiry, path traversal, and more. Benchmarks covering throughput, latency percentiles, payload sizes, and deny-list scaling.
Summary of ChangesHello @dgellow, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers an experimental JWE-based credential proxy designed to enhance security by isolating raw API credentials from potentially compromised sandboxed code. The proxy acts as an intermediary, decrypting encrypted JWE tokens containing credentials and injecting them just before forwarding requests to the target API. This approach ensures that sensitive information is never exposed to the client-side environment, while also providing robust features like host-based access control, key lifecycle management, and token revocation to maintain a secure and flexible credential delivery system. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an impressive initial commit for the stainless-proxy. The project is well-structured, with a clear separation of concerns between proxying, cryptography, key management, and server logic. The inclusion of comprehensive integration tests, benchmarks, and security considerations like memory zeroing for secrets (runtime/secret) from the outset is excellent. My review focuses on a few areas to further enhance security, robustness, and correctness.
| if auth != "Bearer "+string(secret) { | ||
| http.Error(w, `{"error":"unauthorized"}`, http.StatusUnauthorized) | ||
| return | ||
| } |
There was a problem hiding this comment.
Using the != operator for comparing secrets is vulnerable to timing attacks, which could allow an attacker to reconstruct the secret by measuring response time differences. You should use a constant-time comparison function.
Please use subtle.ConstantTimeCompare for this check. You will need to add "crypto/subtle" to your imports.
Note: This approach still leaks the length of the expected token. A more secure (but more complex) alternative is to store a hash of the mint secret and compare the hash of the incoming token against it.
expectedAuth := "Bearer " + string(secret)
// Use constant-time comparison to prevent timing attacks.
if len(auth) != len(expectedAuth) || subtle.ConstantTimeCompare([]byte(auth), []byte(expectedAuth)) != 1 {
http.Error(w, `{"error":"unauthorized"}`, http.StatusUnauthorized)
return
}| if err := os.MkdirAll(keyDir, 0700); err != nil { | ||
| return nil, fmt.Errorf("creating key directory: %w", err) | ||
| } | ||
| path := filepath.Join(keyDir, fmt.Sprintf("key-%s.pem", time.Now().Format("2006-01-02"))) |
There was a problem hiding this comment.
The filename format for generated keys, key-YYYY-MM-DD.pem, is not unique enough. If the service is restarted multiple times on the same day, it will overwrite the key file generated earlier that day. This could lead to an availability issue if an old key that is still in use gets overwritten. Using a more granular, unique timestamp (e.g., including time) would prevent this.
| path := filepath.Join(keyDir, fmt.Sprintf("key-%s.pem", time.Now().Format("2006-01-02"))) | |
| path := filepath.Join(keyDir, fmt.Sprintf("key-%s.pem", time.Now().UTC().Format("20060102T150405Z"))) |
| d.mu.RLock() | ||
| exp, ok := d.entries[hash] | ||
| d.mu.RUnlock() | ||
| if !ok { | ||
| return false | ||
| } | ||
| if time.Now().After(exp) { | ||
| d.mu.Lock() | ||
| delete(d.entries, hash) | ||
| d.mu.Unlock() | ||
| return false | ||
| } | ||
| return true |
There was a problem hiding this comment.
This implementation of IsRevoked has a race condition and is inefficient. It releases the read lock and then takes a write lock to delete an entry. Between the unlock and lock, another goroutine could modify the map, potentially causing a valid token to be deleted.
A safer and more performant approach is to perform the entire check under a single read lock and rely on the periodic Cleanup goroutine to handle the deletion of expired entries. This avoids the complexity and performance cost of lock upgrading on a read-heavy path.
d.mu.RLock()
defer d.mu.RUnlock()
exp, ok := d.entries[hash]
if !ok {
return false
}
// Rely on the periodic cleanup task to remove expired entries.
// This avoids a lock upgrade on a read path.
return time.Now().Before(exp)
stainless-proxy/cmd/mint/main.go
Outdated
| resp, err := http.Get(*jwksURL) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "error: fetching JWKS: %v\n", err) | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
http.Get uses the default HTTP client, which has no timeout. This can cause the CLI tool to hang indefinitely if the JWKS endpoint is slow or unresponsive. It's good practice to use an http.Client with a reasonable timeout.
| resp, err := http.Get(*jwksURL) | |
| if err != nil { | |
| fmt.Fprintf(os.Stderr, "error: fetching JWKS: %v\n", err) | |
| os.Exit(1) | |
| } | |
| client := http.Client{Timeout: 30 * time.Second} | |
| resp, err := client.Get(*jwksURL) | |
| if err != nil { | |
| fmt.Fprintf(os.Stderr, "error: fetching JWKS: %v\n", err) | |
| os.Exit(1) | |
| } |
| if (value[0] == '"' && value[len(value)-1] == '"') || | ||
| (value[0] == '\'' && value[len(value)-1] == '\'') { | ||
| value = value[1 : len(value)-1] | ||
| } |
There was a problem hiding this comment.
This logic to trim quotes from environment variable values can be simplified and made more robust by using strings.Trim. This avoids manual index checking and handles both single and double quotes more cleanly.
value = strings.Trim(value, "\"'")| case "Connection", "Keep-Alive", "Proxy-Authenticate", | ||
| "Proxy-Authorization", "Te", "Trailers", | ||
| "Transfer-Encoding", "Upgrade": | ||
| return true |
There was a problem hiding this comment.
There's a small typo in the list of hop-by-hop headers. According to RFC 2616 and RFC 7230, the header is Trailer, not Trailers. While not a critical issue, correcting it ensures better protocol compliance.
| case "Connection", "Keep-Alive", "Proxy-Authenticate", | |
| "Proxy-Authorization", "Te", "Trailers", | |
| "Transfer-Encoding", "Upgrade": | |
| return true | |
| case "Connection", "Keep-Alive", "Proxy-Authenticate", | |
| "Proxy-Authorization", "Te", "Trailer", | |
| "Transfer-Encoding", "Upgrade": |
…http timeout - Use constant-time comparison for mint secret auth - Make DenyList.IsRevoked read-only to fix race between RUnlock and Lock where a concurrent Add could be deleted; expired entry cleanup is left to the Cleanup goroutine - Add 30s timeout to JWKS fetch in mint CLI - Fix hop-by-hop header name: Trailers → Trailer per RFC 7230
|
Moved to its own repo https://github.com/stainless-api/derouge |
1 participant
Stateless proxy that decrypts JWE tokens and injects raw API credentials at the last hop before the target API. Sandboxes running agent-generated code never see raw credentials. Only encrypted JWEs that are host-locked to prevent exfiltration.
Uses ECDH-ES+A256KW with A256GCM on P-256 keys (go-jose/v4). Includes host locking, key rotation via JWKS, revocation deny-list, per-request tracing, runtime/secret.Do integration for memory zeroing, and manual plaintext zeroing as a cross-platform fallback.