Conversation
…ecation - Replace deprecated proxy.Director with Rewrite (httputil.ReverseProxy) - Add verifyUpstreamSignature() to validate incoming requests were signed by the Agentuity ion proxy using go-common/crypto HTTP signature headers - Signature is read from HTTP trailer; body is buffered and restored for downstream proxying - Verification currently logs warnings only (non-enforcing) pending the ion-side change to send its signing public key via provider.Configuration
… signing key The ion signing transport removes Content-Length to enable streaming signature trailers over HTTP/2. When the gravity dev-mode proxy forwards to the local agent over HTTP/1.1, the missing Content-Length causes chunked transfer encoding which many dev servers cannot parse (400). Buffer the body and restore Content-Length before forwarding. Also wire the signing public key from ion through the Configuration so signature verification is ready to enable once all ion instances consistently derive the same HKDF signing key. Changes: - stack.go: buffer body when Content-Length is -1, restore Content-Length - stack.go: refactor verifyUpstreamSignature to accept pre-buffered body bytes without touching r.Body (verification disabled via TODO) - cmd/start.go: parse SigningPublicKey PEM from Configuration, pass to StartServer - go.mod: bump go-common to v1.0.181
📝 WalkthroughWalkthroughParses an optional upstream ECDSA public key at startup and passes it into StartServer; refactors the reverse proxy to use httputil.ReverseProxy with Rewrite and response modification; adds verifyUpstreamSignature helper and a one-time warning when upstream key is absent but agent has a private key. Changes
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/stack/stack.go (2)
172-175: Comment says "log once" but this logs on everyStartServercall.The warning fires each time
StartServeris invoked (e.g., on reconnection). If the intent is truly once-per-process, consider async.Onceor package-level flag. Otherwise, updating the comment to clarify "once per connection" would be more accurate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/stack/stack.go` around lines 172 - 175, The comment claiming "Log once" is misleading because the logger.Warn call in StartServer (the block checking signingKey == nil && agent.PrivateKey != nil) runs on every StartServer invocation; either make the behavior truly once-per-process by adding a package-level sync.Once or boolean guard and invoke logger.Warn inside that guard (referencing signingKey, agent.PrivateKey and StartServer), or update the comment to accurately state it logs once per StartServer/connection instead of once per process; choose one approach and apply consistently.
215-221: Track this TODO for enabling signature verification.The explanation is clear and well-documented. Once ion-side key consistency is resolved, this verification should be enabled.
Would you like me to open an issue to track enabling upstream signature verification once the ion-side prerequisites are met?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/stack/stack.go` around lines 215 - 221, Create a tracked issue to re-enable upstream signature verification referenced by the TODO in stack.go and the verifyUpstreamSignature() function: describe the prerequisite that all ion instances must derive the same HKDF signing key (include how to detect key divergence), list concrete steps to enable verification (ensure consistent HKDF key derivation, update rolling-deploy procedure, re-enable verifyUpstreamSignature() call, and restore POST body hash checks), add required tests (unit/integration for signature verification and a rolling-deploy integration test), and note rollout criteria and rollback plan for deployments where keys may differ; link the issue to the stack.go TODO and any relevant CI/deployment docs for follow-up.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/start.go`:
- Around line 150-164: The current block around c.SigningPublicKey decodes and
parses the PEM but treats a non-ECDSA parsed key the same as a parse error,
causing a misleading "failed to parse it" warning; change the logic in the
signing key handling (the pem.Decode / x509.ParsePKIXPublicKey path that assigns
signingKey via pub.(*ecdsa.PublicKey)) to differentiate parse failure from type
mismatch: if x509.ParsePKIXPublicKey returns an error log that parse error, but
if parse succeeds and the type assertion to *ecdsa.PublicKey yields nil, log a
clear warning that the key parsed but is the wrong type and include the actual
parsed key type (e.g., via reflect.TypeOf(pub) or fmt.Sprintf("%T", pub));
retain the existing logger.Info when signingKey is set.
In `@internal/stack/stack.go`:
- Around line 202-213: The current use of io.ReadAll on r.Body in the request
handling block (checking r.ContentLength, r.Body and http.NoBody) can OOM;
replace it with a bounded read using io.LimitReader (or similar) with a defined
maxBufferSize constant, read up to that limit, detect if the body exceeded the
limit and return a 413 or 502 error and log via logger.Error; if within limit,
recreate r.Body with bytes.NewReader of the bytes read and set r.ContentLength
to the actual length. Ensure symbols referenced are r.Body, r.ContentLength,
io.ReadAll (replace), io.LimitReader, bytes.NewReader, and logger.Error.
---
Nitpick comments:
In `@internal/stack/stack.go`:
- Around line 172-175: The comment claiming "Log once" is misleading because the
logger.Warn call in StartServer (the block checking signingKey == nil &&
agent.PrivateKey != nil) runs on every StartServer invocation; either make the
behavior truly once-per-process by adding a package-level sync.Once or boolean
guard and invoke logger.Warn inside that guard (referencing signingKey,
agent.PrivateKey and StartServer), or update the comment to accurately state it
logs once per StartServer/connection instead of once per process; choose one
approach and apply consistently.
- Around line 215-221: Create a tracked issue to re-enable upstream signature
verification referenced by the TODO in stack.go and the
verifyUpstreamSignature() function: describe the prerequisite that all ion
instances must derive the same HKDF signing key (include how to detect key
divergence), list concrete steps to enable verification (ensure consistent HKDF
key derivation, update rolling-deploy procedure, re-enable
verifyUpstreamSignature() call, and restore POST body hash checks), add required
tests (unit/integration for signature verification and a rolling-deploy
integration test), and note rollout criteria and rollback plan for deployments
where keys may differ; link the issue to the stack.go TODO and any relevant
CI/deployment docs for follow-up.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2696de45-3c7e-4c7d-84f3-43a1a706de71
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
cmd/start.gogo.modinternal/stack/stack.go
📜 Review details
🔇 Additional comments (4)
go.mod (1)
6-6: Dependency bump matches the PR intent.Updating
github.com/agentuity/go-commontov1.0.181is consistent with the new signing-key and proxy-bridge behavior introduced in this PR.internal/stack/stack.go (2)
145-170: LGTM on the proxy configuration.The switch from the deprecated
DirectortoRewriteis correct. TheX-Forwarded-Hostrestoration ensures the local dev server sees the expected public hostname, and theErrorHandlerproperly suppresses expected context cancellation noise.
254-293: LGTM on the signature verification helper.The function is well-structured: it accepts pre-buffered body bytes to avoid modifying
r.Body, handles both trailer and header signature sources, and provides useful debug logging. Ready for enablement once the ion-side prerequisites are resolved.cmd/start.go (1)
165-165: LGTM on the updatedStartServerinvocation.The call correctly passes the agent metadata and parsed signing key, matching the updated function signature.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/start.go (1)
159-161: Avoid logging this as active verification yet.The PR still leaves upstream signature verification disabled, so Line 161 reads as if the proxy is already enforcing it. Consider logging that the key was loaded instead, and reserve the verification wording for when the check is actually enabled.
💡 Possible wording
- logger.Info("using upstream signing key for proxy verification") + logger.Info("loaded upstream signing key")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/start.go` around lines 159 - 161, The log message currently says verification is active though it only assigns signingKey = ecKey; change the log emitted from logger.Info to state that the upstream signing key was loaded or available (e.g., "loaded upstream signing key for potential verification") so it doesn't imply enforcement; keep the assignment to signingKey and only log that verification is enabled at the actual verification-enabling code path where the verification flag/logic runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/start.go`:
- Around line 159-161: The log message currently says verification is active
though it only assigns signingKey = ecKey; change the log emitted from
logger.Info to state that the upstream signing key was loaded or available
(e.g., "loaded upstream signing key for potential verification") so it doesn't
imply enforcement; keep the assignment to signingKey and only log that
verification is enabled at the actual verification-enabling code path where the
verification flag/logic runs.
Ion PR #380 preserves Content-Length through the signing transport, so gravity no longer needs to buffer the request body in RAM to restore it. Requests pass straight through to the reverse proxy.
The plumbing is in place (signing key parsing, verifyUpstreamSignature function, StartServer parameter) but verification is disabled pending investigation of keyid inconsistencies across ion instances. The Content-Length fix (ion PR #380) is the ship-blocking change — it resolves the 400 Bad Request on POST endpoints.
jhaynie
changed the title
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)
internal/stack/stack.go (1)
182-197:⚠️ Potential issue | 🔴 CriticalWire up the signature verification to restore Content-Length before proxying.
The
verifyUpstreamSignaturefunction exists to read the body, save the originalContent-Length, verify the signature, and restore both—preventing chunked encoding to localhost. However, it is never called in the request handler. Non-health requests go directly toproxy.ServeHTTP(w, r)without any normalization, so requests that arrive withoutContent-Lengthare still forwarded to localhost as chunked and will hit the same 400s this PR is trying to eliminate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/stack/stack.go` around lines 182 - 197, The handler currently forwards non-/_health requests directly to proxy.ServeHTTP without calling verifyUpstreamSignature, so bodies read for signature verification and the original Content-Length restoration never happen; update the request handling in the HTTP handler (the anonymous http.HandlerFunc) to call verifyUpstreamSignature(w, r) before proxy.ServeHTTP and only proxy when verifyUpstreamSignature succeeds (restores Content-Length and body), ensuring verifyUpstreamSignature is invoked for non-health paths and its restored request is passed into proxy.ServeHTTP instead of forwarding the original chunked request.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/stack/stack.go`:
- Around line 246-278: The Content-Length restoration and trailer/header cleanup
around signature verification are incorrect: if origContentLength is -1 (common
for HTTP/2) you must set r.ContentLength to the normalized length
int64(len(body)) instead of restoring -1, and you must remove the signature from
the request before forwarding (strip agcrypto.HeaderSignature from r.Trailer
when the signature source is "trailer" and from r.Header when source is
"header"). Locate the block using origContentLength, body, sig, source,
agcrypto.HeaderSignature and VerifyHTTPRequest and change the post-read logic to
(1) delete the signature from the appropriate place based on source, (2) restore
r.Body to io.NopCloser(bytes.NewReader(body)), and (3) set r.ContentLength =
origContentLength if origContentLength >= 0 else r.ContentLength =
int64(len(body)).
---
Outside diff comments:
In `@internal/stack/stack.go`:
- Around line 182-197: The handler currently forwards non-/_health requests
directly to proxy.ServeHTTP without calling verifyUpstreamSignature, so bodies
read for signature verification and the original Content-Length restoration
never happen; update the request handling in the HTTP handler (the anonymous
http.HandlerFunc) to call verifyUpstreamSignature(w, r) before proxy.ServeHTTP
and only proxy when verifyUpstreamSignature succeeds (restores Content-Length
and body), ensuring verifyUpstreamSignature is invoked for non-health paths and
its restored request is passed into proxy.ServeHTTP instead of forwarding the
original chunked request.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d560d77b-93ea-47a0-b3dd-0600b0b60d47
📒 Files selected for processing (1)
internal/stack/stack.go
📜 Review details
🔇 Additional comments (1)
internal/stack/stack.go (1)
172-175: Good startup-only warning.This surfaces the missing upstream key without turning every request into noise.
| // Read the full body so HTTP trailers become available. Save the | ||
| // original Content-Length so we can restore it after verification — | ||
| // the reverse proxy needs it to avoid chunked encoding to localhost. | ||
| origContentLength := r.ContentLength | ||
| body, err := io.ReadAll(r.Body) | ||
| if err != nil { | ||
| return fmt.Errorf("read body: %w", err) | ||
| } | ||
|
|
||
| // The ion proxy sends the Signature as an HTTP trailer (for streaming | ||
| // requests) or as a header (for WebSocket requests). | ||
| sig := r.Trailer.Get(agcrypto.HeaderSignature) | ||
| source := "trailer" | ||
| if sig == "" { | ||
| sig = r.Header.Get(agcrypto.HeaderSignature) | ||
| source = "header" | ||
| if sig == "" { | ||
| source = "missing" | ||
| } | ||
| } | ||
| logger.Debug("upstream signature value: source=%s present=%v", source, sig != "") | ||
|
|
||
| // Verify the cryptographic signature against the upstream signing key. | ||
| verifyErr := agcrypto.VerifyHTTPRequest(publicKey, r, body, nil) | ||
| if verifyErr != nil { | ||
| logger.Debug("upstream signature verification failed: %v", verifyErr) | ||
| } else { | ||
| logger.Debug("upstream signature verification succeeded") | ||
| } | ||
|
|
||
| // Restore the body and Content-Length so the reverse proxy can forward it. | ||
| r.Body = io.NopCloser(bytes.NewReader(body)) | ||
| r.ContentLength = origContentLength |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cd internal/stack && sed -n '240,285p' stack.goRepository: agentuity/gravity
Length of output: 1568
🏁 Script executed:
# Let's also check the full function context
cd internal/stack && wc -l stack.goRepository: agentuity/gravity
Length of output: 73
🏁 Script executed:
# Find the function name and broader context
cd internal/stack && grep -n "verifyUpstreamSignature\|func.*signature" stack.go | head -20Repository: agentuity/gravity
Length of output: 251
🏁 Script executed:
# Check if there are tests or related usage
rg -n "verifyUpstreamSignature|origContentLength" internal/Repository: agentuity/gravity
Length of output: 430
🏁 Script executed:
# Find where verifyUpstreamSignature is called
rg -n "verifyUpstreamSignature" internal/Repository: agentuity/gravity
Length of output: 299
🏁 Script executed:
# Check for HTTP/2 references in the codebase
rg -n "HTTP/2|h2|:authority|pseudo-header" internal/stack/ | head -20Repository: agentuity/gravity
Length of output: 136
🏁 Script executed:
# Look at the function signature and its callers more broadly
cd internal/stack && sed -n '229,285p' stack.goRepository: agentuity/gravity
Length of output: 2232
🏁 Script executed:
# Check if TransferEncoding is manipulated anywhere in this file
rg -n "TransferEncoding" internal/stack/Repository: agentuity/gravity
Length of output: 43
🏁 Script executed:
# Look for calls to verifyUpstreamSignature
rg -n "verifyUpstreamSignature\(" internal/Repository: agentuity/gravity
Length of output: 193
🏁 Script executed:
# Check the broader stack.go file to see request handling flow
cd internal/stack && sed -n '1,50p' stack.goRepository: agentuity/gravity
Length of output: 1039
🏁 Script executed:
# Search for how requests flow through this code
rg -n "func.*Request\|http\.Request" internal/stack/stack.go | head -10Repository: agentuity/gravity
Length of output: 43
🏁 Script executed:
# Search all files for calls to verifyUpstreamSignature
rg "verifyUpstreamSignature" --type goRepository: agentuity/gravity
Length of output: 291
🏁 Script executed:
# Look for HTTP handler or middleware that might use this
rg -n "ServeHTTP\|Handler\|Middleware" internal/stack/stack.go | head -20Repository: agentuity/gravity
Length of output: 43
🏁 Script executed:
# Check if there's any proxy configuration or request handling
rg -n "ReverseProxy\|Director\|Transport" internal/stack/stack.go | head -20Repository: agentuity/gravity
Length of output: 43
🏁 Script executed:
# Search all go files in the repo for calls
rg "verifyUpstreamSignature\(" . --type goRepository: agentuity/gravity
Length of output: 191
🏁 Script executed:
# Check if the function is dead code or commented out
rg -B5 -A5 "verifyUpstreamSignature\(" internal/stack/stack.goRepository: agentuity/gravity
Length of output: 643
🏁 Script executed:
# Look for HTTP/2 streaming or trailer handling
rg -n "Trailer\|trailer" internal/stack/stack.goRepository: agentuity/gravity
Length of output: 43
🏁 Script executed:
# Check the context after the function - what calls it or how it integrates
cd internal/stack && sed -n '278,350p' stack.goRepository: agentuity/gravity
Length of output: 2080
🏁 Script executed:
# Search more broadly - maybe it's commented out or in a different pattern
rg "verifyUpstream" . --type goRepository: agentuity/gravity
Length of output: 295
🏁 Script executed:
# Check if there are any TODOs or comments about enabling this
rg -n "TODO\|FIXME\|disabled\|commented" internal/stack/stack.go | head -20Repository: agentuity/gravity
Length of output: 43
🏁 Script executed:
# Look at the imports to understand the context better
cd internal/stack && head -40 stack.goRepository: agentuity/gravity
Length of output: 858
🏁 Script executed:
# Check if there are other signature verification patterns
rg -n "VerifyHTTP\|signature" internal/stack/stack.goRepository: agentuity/gravity
Length of output: 43
🏁 Script executed:
# Search for any references or comments about this being disabled/unused
rg -B10 "func verifyUpstreamSignature" internal/stack/stack.goRepository: agentuity/gravity
Length of output: 464
🏁 Script executed:
# Check if there's a git history that might explain the function
git log --oneline -n 50 -- internal/stack/stack.go 2>/dev/null | head -20Repository: agentuity/gravity
Length of output: 114
🏁 Script executed:
# Look for any test files that might reference this function
find . -name "*test*.go" -type f | xargs grep -l "verifyUpstreamSignature" 2>/dev/nullRepository: agentuity/gravity
Length of output: 45
🏁 Script executed:
# Check the full file for context on where this function sits
cd internal/stack && wc -l stack.go && grep -n "^func " stack.goRepository: agentuity/gravity
Length of output: 792
🏁 Script executed:
# Check the git commit that disabled signature verification
git show a9e526c --stat 2>/dev/null | head -30Repository: agentuity/gravity
Length of output: 1438
🏁 Script executed:
# Look for any calls to verifyUpstreamSignature in the actual code flow
rg -n "verifyUpstream" . --type go -A 2 -B 2Repository: agentuity/gravity
Length of output: 798
🏁 Script executed:
# Check StartServer function to see if it calls verifyUpstreamSignature
cd internal/stack && sed -n '136,232p' stack.goRepository: agentuity/gravity
Length of output: 3681
🏁 Script executed:
# Check if there's documentation or comments about HTTP/2 handling
rg -n "HTTP/2\|streaming\|chunked\|TransferEncoding" . --type goRepository: agentuity/gravity
Length of output: 43
🏁 Script executed:
# Look at the agcrypto module to understand VerifyHTTPRequest
rg -n "VerifyHTTPRequest" . --type goRepository: agentuity/gravity
Length of output: 156
🏁 Script executed:
# Check Go's net/http documentation behavior - let's verify ContentLength in HTTP/2
python3 << 'EOF'
# HTTP/2 spec: Content-Length header is optional
# In Go's net/http package, when a request body is read via io.ReadAll
# and the request came from HTTP/2, ContentLength may be -1 (unknown)
# This is well-documented in Go's http.Request struct comments
# Let's verify our understanding about the fix
print("HTTP/2 ContentLength behavior:")
print("- HTTP/2 doesn't require Content-Length header")
print("- Go's http.Request sets ContentLength = -1 when reading streamed/chunked bodies")
print("- After ReadAll(), we know the exact length, so setting ContentLength = len(body) is correct")
print("- Clearing TransferEncoding prevents chunked encoding downstream")
EOFRepository: agentuity/gravity
Length of output: 377
🏁 Script executed:
# Check if there are any other places in the codebase that handle Content-Length restoration
rg -n "ContentLength\|Transfer-Encoding" . --type go -A 2 -B 2Repository: agentuity/gravity
Length of output: 43
🏁 Script executed:
# Let's verify the HTTP module behavior and understand if our analysis is sound
# Check the exact behavior of http.Request.ContentLength
python3 << 'EOF'
# Based on Go's documentation:
# - http.Request.ContentLength is set to -1 for requests without Content-Length header
# - This happens with chunked encoding, streaming, or HTTP/2
# - After io.ReadAll(r.Body), we know the exact length
# - Setting ContentLength to the actual length is the correct fix
# - TransferEncoding should be cleared to prevent proxy from re-chunking
facts = [
"http.Request has ContentLength int64 field",
"ContentLength = -1 means 'unknown length' (chunked/streamed)",
"http.Request.TransferEncoding []string contains encoding names",
"After ReadAll(), body size is known, should set ContentLength = len(body)",
"TransferEncoding should be cleared to prevent downstream chunking"
]
for fact in facts:
print(f"✓ {fact}")
EOFRepository: agentuity/gravity
Length of output: 377
Fix the Content-Length restoration before re-enabling signature verification.
This function is currently disabled (commit a9e526c), but when signature verification is re-enabled, the Content-Length restoration logic needs correction. For HTTP/2 requests, origContentLength is often -1. Restoring that value after ReadAll() reintroduces an unknown-length body, which defeats the normalization and allows the reverse proxy to fall back to chunked transfer encoding. Additionally, if the signature came via HTTP trailer, that must be stripped from the forwarded request.
💡 Required fix for re-enablement
- // Read the full body so HTTP trailers become available. Save the
- // original Content-Length so we can restore it after verification —
- // the reverse proxy needs it to avoid chunked encoding to localhost.
- origContentLength := r.ContentLength
+ // Read the full body so HTTP trailers become available.
body, err := io.ReadAll(r.Body)
if err != nil {
return fmt.Errorf("read body: %w", err)
}
@@
// Restore the body and Content-Length so the reverse proxy can forward it.
r.Body = io.NopCloser(bytes.NewReader(body))
- r.ContentLength = origContentLength
+ r.ContentLength = int64(len(body))
+ r.TransferEncoding = nil📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Read the full body so HTTP trailers become available. Save the | |
| // original Content-Length so we can restore it after verification — | |
| // the reverse proxy needs it to avoid chunked encoding to localhost. | |
| origContentLength := r.ContentLength | |
| body, err := io.ReadAll(r.Body) | |
| if err != nil { | |
| return fmt.Errorf("read body: %w", err) | |
| } | |
| // The ion proxy sends the Signature as an HTTP trailer (for streaming | |
| // requests) or as a header (for WebSocket requests). | |
| sig := r.Trailer.Get(agcrypto.HeaderSignature) | |
| source := "trailer" | |
| if sig == "" { | |
| sig = r.Header.Get(agcrypto.HeaderSignature) | |
| source = "header" | |
| if sig == "" { | |
| source = "missing" | |
| } | |
| } | |
| logger.Debug("upstream signature value: source=%s present=%v", source, sig != "") | |
| // Verify the cryptographic signature against the upstream signing key. | |
| verifyErr := agcrypto.VerifyHTTPRequest(publicKey, r, body, nil) | |
| if verifyErr != nil { | |
| logger.Debug("upstream signature verification failed: %v", verifyErr) | |
| } else { | |
| logger.Debug("upstream signature verification succeeded") | |
| } | |
| // Restore the body and Content-Length so the reverse proxy can forward it. | |
| r.Body = io.NopCloser(bytes.NewReader(body)) | |
| r.ContentLength = origContentLength | |
| // Read the full body so HTTP trailers become available. | |
| body, err := io.ReadAll(r.Body) | |
| if err != nil { | |
| return fmt.Errorf("read body: %w", err) | |
| } | |
| // The ion proxy sends the Signature as an HTTP trailer (for streaming | |
| // requests) or as a header (for WebSocket requests). | |
| sig := r.Trailer.Get(agcrypto.HeaderSignature) | |
| source := "trailer" | |
| if sig == "" { | |
| sig = r.Header.Get(agcrypto.HeaderSignature) | |
| source = "header" | |
| if sig == "" { | |
| source = "missing" | |
| } | |
| } | |
| logger.Debug("upstream signature value: source=%s present=%v", source, sig != "") | |
| // Verify the cryptographic signature against the upstream signing key. | |
| verifyErr := agcrypto.VerifyHTTPRequest(publicKey, r, body, nil) | |
| if verifyErr != nil { | |
| logger.Debug("upstream signature verification failed: %v", verifyErr) | |
| } else { | |
| logger.Debug("upstream signature verification succeeded") | |
| } | |
| // Restore the body and Content-Length so the reverse proxy can forward it. | |
| r.Body = io.NopCloser(bytes.NewReader(body)) | |
| r.ContentLength = int64(len(body)) | |
| r.TransferEncoding = nil |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/stack/stack.go` around lines 246 - 278, The Content-Length
restoration and trailer/header cleanup around signature verification are
incorrect: if origContentLength is -1 (common for HTTP/2) you must set
r.ContentLength to the normalized length int64(len(body)) instead of restoring
-1, and you must remove the signature from the request before forwarding (strip
agcrypto.HeaderSignature from r.Trailer when the signature source is "trailer"
and from r.Header when source is "header"). Locate the block using
origContentLength, body, sig, source, agcrypto.HeaderSignature and
VerifyHTTPRequest and change the post-read logic to (1) delete the signature
from the appropriate place based on source, (2) restore r.Body to
io.NopCloser(bytes.NewReader(body)), and (3) set r.ContentLength =
origContentLength if origContentLength >= 0 else r.ContentLength =
int64(len(body)).
1 participant
Summary
Problem
The ion signing transport removes
Content-Lengthand sets it to-1to enable streaming signature trailers over HTTP/2. When gravity's reverse proxy forwards these requests to the local agent over HTTP/1.1, the missingContent-Lengthcauses chunked transfer encoding. Many dev servers (Vite, Node.js, etc.) cannot parse the chunked body, resulting in 400 Bad Request errors.Solution
Buffer the request body in the handler and restore
Content-Lengthbefore forwarding:Signature Verification (TODO)
The plumbing for upstream signature verification is in place:
cmd/start.goparses the signing public key PEM fromConfigurationStartServeraccepts the signing key parameterverifyUpstreamSignatureis refactored to accept pre-buffered body bytes (never touchesr.Body)Verification is disabled pending:
Files Changed
internal/stack/stack.goverifyUpstreamSignatureto not touchr.Body; verification disabled with TODOcmd/start.goSigningPublicKeyPEM from Configuration, pass toStartServergo.mod/go.sumSummary by CodeRabbit
New Features
Bug Fixes
Chores