feat(turnkey): Integrate Turnkey support for remote signer functionality - #5
feat(turnkey): Integrate Turnkey support for remote signer functionality#5eliteprox wants to merge 9 commits into
Conversation
- Added Turnkey organization ID and API key configuration options to the Livepeer node. - Implemented wallet management API for creating and listing Ethereum wallets and accounts via Turnkey. - Enhanced remote signer capabilities to support multiple Ethereum addresses managed by Turnkey. - Updated CLI to include Turnkey-specific commands for wallet and account management. - Improved documentation on Turnkey mode and its integration with the remote signer. This update allows for secure Ethereum key management using Turnkey, enhancing the flexibility and security of the remote signer node.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis pull request adds Turnkey remote-signer configuration, Ethereum account integration, multi-address signing, wallet-management endpoints, address-book status reporting, remote-signer startup safeguards, and related runtime, discovery, build, and documentation updates. ChangesTurnkey remote signer
Runtime and build tooling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Gateway
participant RemoteSigner
participant LivepeerNode
Gateway->>RemoteSigner: Request signing with optional address
RemoteSigner->>LivepeerNode: Validate address against Turnkey address book
LivepeerNode-->>RemoteSigner: Allow or reject address
RemoteSigner-->>Gateway: Return signed response
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 12
🤖 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/livepeer_cli/wizard_turnkey.go`:
- Around line 18-30: The Turnkey calls use the global http.Get/http.Post which
can hang; add a shared http.Client with a timeout on the wizard (e.g., field
name HTTPClient or httpClient), initialize it when the wizard is created
(time.Duration like 10s), and replace uses of http.Get/http.Post in
turnkeyListWallets (and the other three Turnkey actions) with w.HTTPClient.Get /
w.HTTPClient.Post so all requests are bounded by the configured timeout and
reuse connections.
In `@cmd/livepeer/starter/starter.go`:
- Around line 881-883: Currently code silently picks accts[0] for signAddr when
multiple Turnkey accounts exist; instead, update the startup logic around
signAddr/accts to detect len(accts) > 1 and refuse to pick arbitrarily: if the
CLI flag ethAcctAddr (or persisted default) is provided, load and validate that
address against accts; if not provided and multiple accounts exist, return an
error asking the user to supply -ethAcctAddr (or load a persisted default
selection) rather than selecting accts[0]. Modify the logic that sets signAddr
to consult ethAcctAddr and persisted default before falling back to a
single-account implicit choice.
In `@core/livepeernode.go`:
- Around line 423-433: The TurnkeySigningAddressAllowed function currently
allows all addresses when the in-memory address book is empty; change it to
"fail closed" so that when n.TurnkeyMode is true and n.TurnkeyAccount != nil and
len(n.turnkeyAddrs) == 0 the function returns false (deny) instead of true.
Locate the TurnkeySigningAddressAllowed method and replace the empty-book branch
to return false, keeping the existing nil checks and locking
(turnkeyMu.RLock()/RUnlock()) and the final lookup behavior unchanged; update or
add unit tests to assert the deny-on-empty-book behavior.
In `@doc/remote-signer.md`:
- Line 150: Update the sentence about auto-creating a Turnkey wallet to clarify
it only happens when the operator has not provided the -ethAcctAddr flag;
explicitly state that if -ethAcctAddr is set and no matching Ethereum account
exists, startup exits rather than creating a wallet. Mention the -ethAcctAddr
flag and "Turnkey wallet" so readers can locate related behavior in the
code/config.
In `@eth/turnkey_wallets.go`:
- Around line 114-131: The code uses nextEthereumDerivationIndex then
immediately calls CreateWalletAccounts, which races under concurrent
account-creation; change the flow in the function that calls
nextEthereumDerivationIndex/CreateWalletAccounts to serialize or retry on
conflict: either acquire a per-wallet lock (keyed by walletID) around computing
idx and calling c.V0().Wallets.CreateWalletAccounts (release after response) OR
wrap the read-then-write in a retry loop that calls nextEthereumDerivationIndex,
builds the request (path := fmt.Sprintf(..., idx)), calls CreateWalletAccounts,
and if the API returns a duplicate/path-conflict error then re-fetch accounts
(via the same client) and retry up to a small max attempts before returning
error; update referencing symbols nextEthereumDerivationIndex,
CreateWalletAccounts, wallets.NewCreateWalletAccountsParams, and
ethereumWalletAccountParams accordingly.
- Around line 67-72: The Turnkey address strings are being passed directly to
ethcommon.HexToAddress (e.g., when building TurnkeyWalletAccount entries) which
silently accepts malformed input; add explicit validation using
ethcommon.IsHexAddress before each HexToAddress call (the three locations
handling response address fields such as a.Address) and handle failures by
returning an error (or skipping the entry with a logged warning) so malformed
addresses are rejected rather than converted into zero/unintended addresses;
update the code paths that construct TurnkeyWalletAccount (and any similar
structs at the other two sites) to perform the IsHexAddress guard and branch on
invalid input accordingly.
In `@eth/turnkey.go`:
- Around line 54-80: The code mutates shared TurnkeyAccountManager.signWith
(guarded by mu) to set a global signing address via
WithSigningAddress/SetSigningAddress which allows concurrent requests to leak
and mismatch addresses (affecting CreateTransactOpts and SignTx); instead,
remove reliance on a mutable global signWith and make the signing address
explicit per-operation: change WithSigningAddress to pass the desired addr into
fn (e.g., fn(addr) or attach it to a request-scoped context value) so callers
supply the address to CreateTransactOpts/SignTx directly, update
CreateTransactOpts and SignTx to accept an explicit address parameter (or read
it from the provided context) and stop reading/writing signWith, and delete or
deprecate SetSigningAddress; preserve mu for any remaining shared state but
ensure sign selection is per-call not global.
- Around line 172-180: The code dereferences nested Turnkey response fields
(resp.Payload.Activity.Result.SignRawPayloadResult and its R/S/V) without
guarding against nil, which can panic on partial/malformed SDK responses; modify
the SignRawPayload call handling (the block around
t.client.V0().Signing.SignRawPayload and the analogous block at lines 319-327)
to nil-check each level (resp, resp.Payload, resp.Payload.Activity,
resp.Payload.Activity.Result, resp.Payload.Activity.Result.SignRawPayloadResult,
and the R/S/V pointers) and return a descriptive error if any are nil instead of
dereferencing them. Ensure both signing paths perform the same defensive checks
and return an error like "turnkey SignRawPayload: missing response fields" when
validation fails.
In `@server/remote_signer.go`:
- Around line 320-350: The handler allows accepting a foreign req.SignerAddress
when TurnkeyAccount is nil, causing signState() to still use the node's
Eth.Account().Address and producing invalid state signatures; change the logic
in the request handling around effectiveSigner/state.SignerAddress (the block
that reads req.SignerAddress, computes effectiveSigner, and later conditionally
sets state.SignerAddress) to reject requests that supply a different signer than
the node's own account when ls.LivepeerNode.TurnkeyAccount == nil by returning a
400 error via respondJsonError rather than assigning state.SignerAddress; keep
the existing Turnkey path (using TurnkeyAccount, TurnkeySigningAddressAllowed,
SetSigningAddress/ defer restore) unchanged so only non-turnkey nodes enforce
this rejection.
- Around line 39-48: The code currently mutates shared TurnkeyAccount state via
LivepeerNode.TurnkeyAccount by calling SetSigningAddress and deferring a restore
(SigningAddress/SetSigningAddress), which can race across requests; instead
either (A) add a signing API that takes the desired address and performs signing
without changing shared state (e.g., implement
TurnkeyAccount.SignWithAddress(addr, payload) and call that here after
validating TurnkeySigningAddressAllowed), or (B) if changing the account API is
not feasible, protect the SetSigningAddress → sign → restore sequence with a
single global mutex used by every Turnkey signing path (e.g.,
turnkeySignerMutex) so the mutation is serialized; apply the same change to the
other occurrence noted (lines ~343-350) and remove any defer-based state swaps
so no window exists where the process-wide signer is wrong.
- Around line 221-227: Only allow state.SignerAddress to override the
verification address when Turnkey mode is active: change the logic that sets
addr so it only uses state.SignerAddress if ls.LivepeerNode.TurnkeyAccount !=
nil (i.e., Turnkey mode enabled) and state != nil && state.SignerAddress != "";
keep the default addr from ls.LivepeerNode.Eth.Account().Address for non‑Turnkey
mode and continue to use ls.LivepeerNode.TurnkeySigningAddressAllowed(addr) to
validate the chosen addr.
In `@server/turnkey_http.go`:
- Around line 75-81: The create handlers in turnkey_http.go call
ls.refreshTurnkeyAddressBook(ctx) but ignore its error, returning success even
if the allowlist update failed; update each create endpoint that calls
ls.refreshTurnkeyAddressBook to check its returned error, and if non-nil respond
with an error status (e.g., http.StatusInternalServerError) and a JSON error
body instead of the success payload; keep Content-Type header logic, log the
error via glog (including context like walletId/address) and return early on
error so the caller only receives a success response when
refreshTurnkeyAddressBook succeeds (apply the same change to every occurrence of
ls.refreshTurnkeyAddressBook in this file).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc3c64a1-7ef6-4d99-abc3-70cadcdb59bc
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (18)
cmd/livepeer/starter/flags.gocmd/livepeer/starter/starter.gocmd/livepeer_cli/livepeer_cli.gocmd/livepeer_cli/wizard_turnkey.gocommon/types.gocore/livepeernode.godoc/remote-signer.mdeth/turnkey.goeth/turnkey_test.goeth/turnkey_wallets.gogo.modserver/cliserver_test.goserver/live_payment.goserver/mediaserver.goserver/remote_signer.goserver/remote_signer_test.goserver/turnkey_http.goserver/webserver.go
| func (w *wizard) turnkeyListWallets() { | ||
| resp, err := http.Get(w.turnkeyBase() + "/turnkey/wallets") | ||
| if err != nil { | ||
| log.Error("Turnkey list wallets failed", "err", err) | ||
| return | ||
| } | ||
| defer resp.Body.Close() | ||
| body, _ := io.ReadAll(resp.Body) | ||
| if resp.StatusCode != http.StatusOK { | ||
| log.Error("Turnkey list wallets", "status", resp.StatusCode, "body", string(body)) | ||
| return | ||
| } | ||
| fmt.Println(string(body)) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Go's net/http package, what timeout do http.Get/http.Post use by default when no custom http.Client timeout is set?
💡 Result:
In Go's net/http package, http.Get and http.Post use http.DefaultClient by default when no custom http.Client is provided. The DefaultClient is defined as var DefaultClient = &Client{}, which is the zero value of Client. The Client struct has a Timeout field of type time.Duration, and the documentation explicitly states: "A Timeout of zero means no timeout." The client.do method checks if c.Timeout > 0 before applying any deadline; since it is zero, no timeout is set. This behavior is consistent across Go versions, including recent ones like go1.25.1, and is confirmed by multiple secondary sources warning that the default client has no timeout, allowing requests to hang indefinitely.
Citations:
- 1: https://pkg.go.dev/net/http@go1.25.1
- 2: https://pkg.go.dev/net/http@go1.21.5
- 3: https://go.dev/src/net/http/client.go
- 4: https://pkg.go.dev/net/http
- 5: https://github.com/golang/go/blob/master/src/net/http/client.go
- 6: https://golang.org/src/net/http/client.go
- 7: https://go.dev/src/net/http/client.go?s=708:1583
- 8: https://github.com/golang/go/blob/go1.20.12/src/net/http/client.go
- 9: https://go.dev/src/net/http/client.go?m=text
🏁 Script executed:
# Find the file and check its size
fd wizard_turnkey.goRepository: eliteprox/go-livepeer
Length of output: 99
🏁 Script executed:
# Read the entire file to see all HTTP calls
cat -n cmd/livepeer_cli/wizard_turnkey.goRepository: eliteprox/go-livepeer
Length of output: 3139
Add a bounded HTTP client for Turnkey wizard calls.
All four actions use the default http.Get/http.Post path, so a broken local socket can hang the CLI indefinitely. Please route these through a shared http.Client with a timeout.
🔧 Minimal fix sketch
import (
"bytes"
"encoding/json"
"fmt"
"io"
"net/http"
"strings"
+ "time"
"github.com/ethereum/go-ethereum/log"
)
+var turnkeyHTTPClient = &http.Client{Timeout: 10 * time.Second}
+
func (w *wizard) turnkeyListWallets() {
- resp, err := http.Get(w.turnkeyBase() + "/turnkey/wallets")
+ resp, err := turnkeyHTTPClient.Get(w.turnkeyBase() + "/turnkey/wallets")
if err != nil {
log.Error("Turnkey list wallets failed", "err", err)
return
}
@@
- resp, err := http.Post(w.turnkeyBase()+"/turnkey/create-wallet", "application/json", bytes.NewReader(payload))
+ resp, err := turnkeyHTTPClient.Post(w.turnkeyBase()+"/turnkey/create-wallet", "application/json", bytes.NewReader(payload))
@@
- resp, err := http.Post(w.turnkeyBase()+"/turnkey/create-account", "application/json", bytes.NewReader(payload))
+ resp, err := turnkeyHTTPClient.Post(w.turnkeyBase()+"/turnkey/create-account", "application/json", bytes.NewReader(payload))
@@
- resp, err := http.Post(w.turnkeyBase()+"/turnkey/select-address", "application/json", bytes.NewReader(payload))
+ resp, err := turnkeyHTTPClient.Post(w.turnkeyBase()+"/turnkey/select-address", "application/json", bytes.NewReader(payload))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/livepeer_cli/wizard_turnkey.go` around lines 18 - 30, The Turnkey calls
use the global http.Get/http.Post which can hang; add a shared http.Client with
a timeout on the wizard (e.g., field name HTTPClient or httpClient), initialize
it when the wizard is created (time.Duration like 10s), and replace uses of
http.Get/http.Post in turnkeyListWallets (and the other three Turnkey actions)
with w.HTTPClient.Get / w.HTTPClient.Post so all requests are bounded by the
configured timeout and reuse connections.
| } else if len(accts) > 0 { | ||
| signAddr = accts[0].Address | ||
| } else { |
There was a problem hiding this comment.
Don't silently pick accts[0] when multiple Turnkey accounts exist.
This makes the signer identity restart-dependent and also loses any address selected at runtime via the Turnkey API. If more than one Ethereum account exists, require -ethAcctAddr or load a persisted default instead of relying on API order.
🔧 Safer startup behavior
- } else if len(accts) > 0 {
- signAddr = accts[0].Address
+ } else if len(accts) == 1 {
+ signAddr = accts[0].Address
+ } else if len(accts) > 1 {
+ glog.Exit("multiple Turnkey Ethereum accounts found; set -ethAcctAddr to choose the signing address")
} else {📝 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.
| } else if len(accts) > 0 { | |
| signAddr = accts[0].Address | |
| } else { | |
| } else if len(accts) == 1 { | |
| signAddr = accts[0].Address | |
| } else if len(accts) > 1 { | |
| glog.Exit("multiple Turnkey Ethereum accounts found; set -ethAcctAddr to choose the signing address") | |
| } else { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/livepeer/starter/starter.go` around lines 881 - 883, Currently code
silently picks accts[0] for signAddr when multiple Turnkey accounts exist;
instead, update the startup logic around signAddr/accts to detect len(accts) > 1
and refuse to pick arbitrarily: if the CLI flag ethAcctAddr (or persisted
default) is provided, load and validate that address against accts; if not
provided and multiple accounts exist, return an error asking the user to supply
-ethAcctAddr (or load a persisted default selection) rather than selecting
accts[0]. Modify the logic that sets signAddr to consult ethAcctAddr and
persisted default before falling back to a single-account implicit choice.
| func (n *LivepeerNode) TurnkeySigningAddressAllowed(addr ethcommon.Address) bool { | ||
| if n == nil || !n.TurnkeyMode || n.TurnkeyAccount == nil { | ||
| return true | ||
| } | ||
| n.turnkeyMu.RLock() | ||
| defer n.turnkeyMu.RUnlock() | ||
| if len(n.turnkeyAddrs) == 0 { | ||
| return true | ||
| } | ||
| _, ok := n.turnkeyAddrs[addr] | ||
| return ok |
There was a problem hiding this comment.
Fail closed when the Turnkey address book is empty.
Once Turnkey mode is active, returning true on len(n.turnkeyAddrs) == 0 turns any address-book load/refresh failure into allow-all behavior for address selection and request routing. This should reject until the book is populated.
🔒 Suggested change
func (n *LivepeerNode) TurnkeySigningAddressAllowed(addr ethcommon.Address) bool {
if n == nil || !n.TurnkeyMode || n.TurnkeyAccount == nil {
return true
}
n.turnkeyMu.RLock()
defer n.turnkeyMu.RUnlock()
if len(n.turnkeyAddrs) == 0 {
- return true
+ return false
}
_, ok := n.turnkeyAddrs[addr]
return ok
}📝 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.
| func (n *LivepeerNode) TurnkeySigningAddressAllowed(addr ethcommon.Address) bool { | |
| if n == nil || !n.TurnkeyMode || n.TurnkeyAccount == nil { | |
| return true | |
| } | |
| n.turnkeyMu.RLock() | |
| defer n.turnkeyMu.RUnlock() | |
| if len(n.turnkeyAddrs) == 0 { | |
| return true | |
| } | |
| _, ok := n.turnkeyAddrs[addr] | |
| return ok | |
| func (n *LivepeerNode) TurnkeySigningAddressAllowed(addr ethcommon.Address) bool { | |
| if n == nil || !n.TurnkeyMode || n.TurnkeyAccount == nil { | |
| return true | |
| } | |
| n.turnkeyMu.RLock() | |
| defer n.turnkeyMu.RUnlock() | |
| if len(n.turnkeyAddrs) == 0 { | |
| return false | |
| } | |
| _, ok := n.turnkeyAddrs[addr] | |
| return ok | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/livepeernode.go` around lines 423 - 433, The
TurnkeySigningAddressAllowed function currently allows all addresses when the
in-memory address book is empty; change it to "fail closed" so that when
n.TurnkeyMode is true and n.TurnkeyAccount != nil and len(n.turnkeyAddrs) == 0
the function returns false (deny) instead of true. Locate the
TurnkeySigningAddressAllowed method and replace the empty-book branch to return
false, keeping the existing nil checks and locking (turnkeyMu.RLock()/RUnlock())
and the final lookup behavior unchanged; update or add unit tests to assert the
deny-on-empty-book behavior.
| - **`-turnkeyOrg <organizationId>`** — Turnkey organization ID. **Must** be used with `-remoteSigner`. When set, the node uses Turnkey for all signing and skips the local keystore. The standard chain flags (`-ethUrl`, `-network`, `-ethController`) are still required. | ||
| - **`-turnkeyApiKeyName <name>`** — Name of the API key loaded from `~/.turnkey/keys/<name>/` (default: `default`). Follows the [tkhq/go-sdk](https://github.com/tkhq/go-sdk) key-store convention. | ||
|
|
||
| If the organization has no Ethereum accounts on first start, the node automatically creates a Turnkey wallet and logs its address. |
There was a problem hiding this comment.
Tighten this startup behavior description.
This is only true when -ethAcctAddr is not set. If an operator pins -ethAcctAddr and the org has no matching Ethereum account, startup exits instead of auto-creating one.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc/remote-signer.md` at line 150, Update the sentence about auto-creating a
Turnkey wallet to clarify it only happens when the operator has not provided the
-ethAcctAddr flag; explicitly state that if -ethAcctAddr is set and no matching
Ethereum account exists, startup exits rather than creating a wallet. Mention
the -ethAcctAddr flag and "Turnkey wallet" so readers can locate related
behavior in the code/config.
| out = append(out, TurnkeyWalletAccount{ | ||
| OrganizationID: orgID, | ||
| WalletID: wid, | ||
| AccountID: aid, | ||
| Address: ethcommon.HexToAddress(*a.Address), | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the turnkey_wallets.go file
find . -name "turnkey_wallets.go" -type fRepository: eliteprox/go-livepeer
Length of output: 89
🏁 Script executed:
# Get the file size to understand if we can read it entirely
wc -l eth/turnkey_wallets.go 2>/dev/null || find . -name "turnkey_wallets.go" -exec wc -l {} \;Repository: eliteprox/go-livepeer
Length of output: 91
🏁 Script executed:
# Search for the specific code sections mentioned in the review
rg "HexToAddress" --context 3 -nRepository: eliteprox/go-livepeer
Length of output: 50377
🏁 Script executed:
# Read the relevant sections of the file
sed -n '60,75p' eth/turnkey_wallets.go
echo "---"
sed -n '100,110p' eth/turnkey_wallets.go
echo "---"
sed -n '135,145p' eth/turnkey_wallets.goRepository: eliteprox/go-livepeer
Length of output: 1553
🏁 Script executed:
# Check if IsHexAddress is used anywhere in the codebase
rg "IsHexAddress" -n --context 2Repository: eliteprox/go-livepeer
Length of output: 864
🏁 Script executed:
# Check go-ethereum common package documentation for HexToAddress behavior
rg -A 5 "HexToAddress.*=.*func" --type goRepository: eliteprox/go-livepeer
Length of output: 47
🏁 Script executed:
# Search for how IsHexAddress is being used for validation
rg "IsHexAddress" -B 3 -A 3Repository: eliteprox/go-livepeer
Length of output: 1017
🏁 Script executed:
# Check if there's any documentation or comment about HexToAddress behavior
rg "HexToAddress" server/turnkey_http.go -B 2 -A 2Repository: eliteprox/go-livepeer
Length of output: 254
🏁 Script executed:
# Look at the pattern where turnkey addresses are being used - check turnkey_http.go which seems to handle addresses
cat -n server/turnkey_http.go | sed -n '125,140p'Repository: eliteprox/go-livepeer
Length of output: 692
🏁 Script executed:
# Search for go.mod to understand which version of go-ethereum is used
cat go.mod | grep ethereumRepository: eliteprox/go-livepeer
Length of output: 156
🏁 Script executed:
# Check if there are any comments or documentation about address validation in the codebase
rg "HexToAddress.*malformed\|HexToAddress.*permissive\|zero.*pad\|truncate" -iRepository: eliteprox/go-livepeer
Length of output: 47
🏁 Script executed:
# Look at the broader context of turnkey_wallets.go to understand data flow
cat -n eth/turnkey_wallets.go | head -80Repository: eliteprox/go-livepeer
Length of output: 2961
🏁 Script executed:
# Search web for HexToAddress behavior in go-ethereum v1.13.5
# First check if there's any test or usage pattern that shows its behavior
rg "HexToAddress.*\(.*\".*\)" -A 1 | head -50Repository: eliteprox/go-livepeer
Length of output: 2274
🏁 Script executed:
# Check the specific pattern in turnkey_http.go where address validation is done
# to understand if the zero-value check is the standard validation approach
cat -n server/turnkey_http.go | sed -n '120,145p'Repository: eliteprox/go-livepeer
Length of output: 1430
🏁 Script executed:
# Look for any other address validation patterns in the codebase
rg "addr.*==.*Address\{\}" -B 2 -A 2 | head -40Repository: eliteprox/go-livepeer
Length of output: 907
🏁 Script executed:
# Search for any documentation or tests about HexToAddress behavior with invalid input
rg -A 5 -B 5 "HexToAddress\(.*foo"Repository: eliteprox/go-livepeer
Length of output: 8892
🏁 Script executed:
# Check if there's any validation of Turnkey API responses before using HexToAddress
cat -n eth/turnkey_wallets.go | sed -n '36,75p'Repository: eliteprox/go-livepeer
Length of output: 1367
🏁 Script executed:
# Let me verify the exact code pattern - check what happens with invalid addresses
# by looking at test patterns with "foo"
rg "HexToAddress.*foo" -B 3 -A 3 | head -30Repository: eliteprox/go-livepeer
Length of output: 1297
🏁 Script executed:
# Let me check what the actual result of HexToAddress("foo") would be
# by looking at test assertions with it
rg "HexToAddress\(.*foo.*\)" -A 5 -B 5 | head -50Repository: eliteprox/go-livepeer
Length of output: 2554
🏁 Script executed:
# Check if there's any explicit validation of Turnkey API response addresses
# Look for address format validation in turnkey-related code
rg "Address.*format\|format.*Address" -i eth/turnkey_wallets.go -B 2 -A 2Repository: eliteprox/go-livepeer
Length of output: 47
🏁 Script executed:
# Search web for go-ethereum HexToAddress behavior documentationRepository: eliteprox/go-livepeer
Length of output: 47
🌐 Web query:
go-ethereum HexToAddress behavior malformed input v1.13.5
💡 Result:
In go-ethereum (including v1.13.5), common.HexToAddress does NOT validate “malformed” input. It always returns an Address value by hex-decoding with cropping semantics, so invalid strings can silently become some other address (or zero / partial bytes). What it does (core behavior) 1) HexToAddress is just a thin wrapper: - HexToAddress(s string) Address calls BytesToAddress(FromHex(s)). 2) FromHex(s string) returns bytes for the hex string, and if the input length is odd it left-pads with a leading '0'. It also accepts an optional 0x prefix. 3) BytesToAddress(b []byte) crops to 20 bytes (the AddressLength = 20) by keeping only the last 20 bytes (left-cropping happens in SetBytes). So malformed inputs: - with wrong length (not 40 hex chars after the optional 0x) are not rejected; instead they are decoded and then cropped/padded to 20 bytes. - with non-hex characters: hex.DecodeString failure is ignored in Hex2Bytes (it returns h, _ := hex.DecodeString(str) and discards the error). As a result, the returned bytes may be empty/partial depending on the decode behavior, and then BytesToAddress will still produce an Address (often effectively “zero” if empty). Implication for “malformed input v1.13.5” - HexToAddress never returns an error, so you cannot detect malformed input using it. - If you need strict validation, use common.IsHexAddress (checks 40 hex chars with optional 0x prefix) and/or the JSON/Text unmarshaling paths that use hexutil.UnmarshalFixedText / related functions, which do return errors on invalid syntax/length. Practical recommendation - For user-supplied addresses, validate first with common.IsHexAddress(s). - If you’re parsing from JSON/RPC where go-ethereum expects fixed-size hex types, rely on the hexutil “UnmarshalFixedText/JSON” implementations (they enforce 0x prefix, even length, exact length, and syntax), rather than HexToAddress. Source-backed references - HexToAddress implementation and the “cropped from the left” behavior for BytesToAddress/HexToAddress are in go-ethereum common/types.go. - FromHex and Hex2Bytes ignore decoding errors and implement odd-length padding behavior in go-ethereum common/bytes.go. - Strict hex decoding + erroring behavior is in go-ethereum common/hexutil (Decode/UnmarshalFixedText).
Citations:
- 1: https://github.com/ethereum/go-ethereum/blob/master/common/types.go
- 2: https://github.com/ethereum/go-ethereum/blob/master/common/bytes.go
- 3: https://pkg.go.dev/github.com/ethereum/go-ethereum@v1.10.20/common/hexutil
- 4: https://pkg.go.dev/github.com/Ethereum/go-ethereum/common
🏁 Script executed:
# Verify the exact behavior by checking if the code has any existing validation
# Check if there's any validation of the address strings before they reach HexToAddress
cat -n eth/turnkey_wallets.go | sed -n '50,75p'Repository: eliteprox/go-livepeer
Length of output: 805
🏁 Script executed:
# Check the Turnkey API response model to understand what format Address comes in
rg "models.*Account\|type.*Account" eth/turnkey_wallets.go -B 2 -A 5Repository: eliteprox/go-livepeer
Length of output: 47
🏁 Script executed:
# Look for any test cases that might show expected address format from Turnkey
find . -name "*turnkey*test*" -type fRepository: eliteprox/go-livepeer
Length of output: 86
🏁 Script executed:
# Check the turnkey_test.go to see how addresses are handled in tests
cat -n eth/turnkey_test.go | head -50Repository: eliteprox/go-livepeer
Length of output: 2015
🏁 Script executed:
# Look at the second and third occurrences more carefully
cat -n eth/turnkey_wallets.go | sed -n '95,150p'Repository: eliteprox/go-livepeer
Length of output: 2846
Validate Turnkey address strings before converting them.
ethcommon.HexToAddress silently handles malformed input instead of rejecting it—it hex-decodes with cropping/padding semantics, meaning invalid strings can become zero addresses or unintended addresses. At lines 71, 106, and 142, Turnkey API response addresses are converted without format validation. Add ethcommon.IsHexAddress checks before each call to HexToAddress to fail fast on malformed data.
🛡️ Suggested guard
- return *res.WalletID, ethcommon.HexToAddress(res.Addresses[0]), nil
+ if !ethcommon.IsHexAddress(res.Addresses[0]) {
+ return "", ethcommon.Address{}, fmt.Errorf("turnkey CreateWallet: invalid address %q", res.Addresses[0])
+ }
+ return *res.WalletID, ethcommon.HexToAddress(res.Addresses[0]), nilApply the same check to lines 71 and 142.
Also applies to: 71, 142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eth/turnkey_wallets.go` around lines 67 - 72, The Turnkey address strings are
being passed directly to ethcommon.HexToAddress (e.g., when building
TurnkeyWalletAccount entries) which silently accepts malformed input; add
explicit validation using ethcommon.IsHexAddress before each HexToAddress call
(the three locations handling response address fields such as a.Address) and
handle failures by returning an error (or skipping the entry with a logged
warning) so malformed addresses are rejected rather than converted into
zero/unintended addresses; update the code paths that construct
TurnkeyWalletAccount (and any similar structs at the other two sites) to perform
the IsHexAddress guard and branch on invalid input accordingly.
| resp, err := t.client.V0().Signing.SignRawPayload(params, t.client.Authenticator) | ||
| if err != nil { | ||
| return "", "", "", err | ||
| } | ||
| res := resp.Payload.Activity.Result.SignRawPayloadResult | ||
| if res == nil || res.R == nil || res.S == nil || res.V == nil { | ||
| return "", "", "", fmt.Errorf("turnkey SignRawPayload: missing r/s/v in response") | ||
| } | ||
| return *res.R, *res.S, *res.V, nil |
There was a problem hiding this comment.
Guard the nested Turnkey response before dereferencing it.
Both signing paths assume resp.Payload.Activity.Result... is fully populated. A partial or malformed SDK response will panic the process here instead of returning an error.
🛡️ Minimal nil-guard fix
resp, err := t.client.V0().Signing.SignRawPayload(params, t.client.Authenticator)
if err != nil {
return "", "", "", err
}
- res := resp.Payload.Activity.Result.SignRawPayloadResult
+ if resp == nil || resp.Payload == nil || resp.Payload.Activity == nil || resp.Payload.Activity.Result == nil {
+ return "", "", "", fmt.Errorf("turnkey SignRawPayload: empty activity result")
+ }
+ res := resp.Payload.Activity.Result.SignRawPayloadResult
if res == nil || res.R == nil || res.S == nil || res.V == nil {
return "", "", "", fmt.Errorf("turnkey SignRawPayload: missing r/s/v in response")
}
@@
resp, err := t.client.V0().Signing.SignTransaction(params, t.client.Authenticator)
if err != nil {
return nil, err
}
- if resp.Payload.Activity.Result.SignTransactionResult == nil ||
- resp.Payload.Activity.Result.SignTransactionResult.SignedTransaction == nil {
+ if resp == nil || resp.Payload == nil || resp.Payload.Activity == nil || resp.Payload.Activity.Result == nil ||
+ resp.Payload.Activity.Result.SignTransactionResult == nil ||
+ resp.Payload.Activity.Result.SignTransactionResult.SignedTransaction == nil {
return nil, fmt.Errorf("turnkey SignTransaction: missing signedTransaction")
}Also applies to: 319-327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@eth/turnkey.go` around lines 172 - 180, The code dereferences nested Turnkey
response fields (resp.Payload.Activity.Result.SignRawPayloadResult and its
R/S/V) without guarding against nil, which can panic on partial/malformed SDK
responses; modify the SignRawPayload call handling (the block around
t.client.V0().Signing.SignRawPayload and the analogous block at lines 319-327)
to nil-check each level (resp, resp.Payload, resp.Payload.Activity,
resp.Payload.Activity.Result, resp.Payload.Activity.Result.SignRawPayloadResult,
and the R/S/V pointers) and return a descriptive error if any are nil instead of
dereferencing them. Ensure both signing paths perform the same defensive checks
and return an error like "turnkey SignRawPayload: missing response fields" when
validation fails.
| if tk := ls.LivepeerNode.TurnkeyAccount; tk != nil { | ||
| if q := strings.TrimSpace(r.URL.Query().Get("address")); q != "" { | ||
| addr := ethcommon.HexToAddress(q) | ||
| if !ls.LivepeerNode.TurnkeySigningAddressAllowed(addr) { | ||
| respondJsonError(ctx, w, fmt.Errorf("unknown or disallowed signing address"), http.StatusBadRequest) | ||
| return | ||
| } | ||
| prev := tk.SigningAddress() | ||
| tk.SetSigningAddress(addr) | ||
| defer tk.SetSigningAddress(prev) |
There was a problem hiding this comment.
Do not switch the active Turnkey signer through shared process state.
Lines 46-48 and 348-350 mutate tk for the whole node, then restore it with defer. Concurrent requests can interleave inside that window and sign with the wrong address. Please make address selection part of the signing call itself, or serialize the entire set → sign → restore sequence with a mutex shared by every Turnkey signing path.
Also applies to: 343-350
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/remote_signer.go` around lines 39 - 48, The code currently mutates
shared TurnkeyAccount state via LivepeerNode.TurnkeyAccount by calling
SetSigningAddress and deferring a restore (SigningAddress/SetSigningAddress),
which can race across requests; instead either (A) add a signing API that takes
the desired address and performs signing without changing shared state (e.g.,
implement TurnkeyAccount.SignWithAddress(addr, payload) and call that here after
validating TurnkeySigningAddressAllowed), or (B) if changing the account API is
not feasible, protect the SetSigningAddress → sign → restore sequence with a
single global mutex used by every Turnkey signing path (e.g.,
turnkeySignerMutex) so the mutation is serialized; apply the same change to the
other occurrence noted (lines ~343-350) and remove any defer-based state swaps
so no window exists where the process-wide signer is wrong.
| addr := ls.LivepeerNode.Eth.Account().Address | ||
| if state != nil && state.SignerAddress != "" { | ||
| addr = ethcommon.HexToAddress(state.SignerAddress) | ||
| } | ||
| if ls.LivepeerNode.TurnkeyAccount != nil && !ls.LivepeerNode.TurnkeySigningAddressAllowed(addr) { | ||
| return fmt.Errorf("signer address not allowed for Turnkey remote signer") | ||
| } |
There was a problem hiding this comment.
Only honor state.SignerAddress when Turnkey mode is enabled.
In non-Turnkey mode, Lines 222-224 let untrusted JSON choose the verification address. That allows a caller to self-sign a forged state and bypass the intended “must be signed by the remote signer account” guarantee before fields like Balance, PMSessionID, and InitialPricePerUnit are trusted.
🔒 Proposed fix
addr := ls.LivepeerNode.Eth.Account().Address
- if state != nil && state.SignerAddress != "" {
+ if ls.LivepeerNode.TurnkeyAccount != nil && state != nil && state.SignerAddress != "" {
addr = ethcommon.HexToAddress(state.SignerAddress)
}📝 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.
| addr := ls.LivepeerNode.Eth.Account().Address | |
| if state != nil && state.SignerAddress != "" { | |
| addr = ethcommon.HexToAddress(state.SignerAddress) | |
| } | |
| if ls.LivepeerNode.TurnkeyAccount != nil && !ls.LivepeerNode.TurnkeySigningAddressAllowed(addr) { | |
| return fmt.Errorf("signer address not allowed for Turnkey remote signer") | |
| } | |
| addr := ls.LivepeerNode.Eth.Account().Address | |
| if ls.LivepeerNode.TurnkeyAccount != nil && state != nil && state.SignerAddress != "" { | |
| addr = ethcommon.HexToAddress(state.SignerAddress) | |
| } | |
| if ls.LivepeerNode.TurnkeyAccount != nil && !ls.LivepeerNode.TurnkeySigningAddressAllowed(addr) { | |
| return fmt.Errorf("signer address not allowed for Turnkey remote signer") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/remote_signer.go` around lines 221 - 227, Only allow
state.SignerAddress to override the verification address when Turnkey mode is
active: change the logic that sets addr so it only uses state.SignerAddress if
ls.LivepeerNode.TurnkeyAccount != nil (i.e., Turnkey mode enabled) and state !=
nil && state.SignerAddress != ""; keep the default addr from
ls.LivepeerNode.Eth.Account().Address for non‑Turnkey mode and continue to use
ls.LivepeerNode.TurnkeySigningAddressAllowed(addr) to validate the chosen addr.
| if q := strings.TrimSpace(req.SignerAddress); q != "" { | ||
| state.SignerAddress = ethcommon.HexToAddress(q).Hex() | ||
| } else { | ||
| state.SignerAddress = ls.LivepeerNode.Eth.Account().Address.Hex() | ||
| } | ||
| } | ||
|
|
||
| effectiveSigner := ls.LivepeerNode.Eth.Account().Address | ||
| if state.SignerAddress != "" { | ||
| effectiveSigner = ethcommon.HexToAddress(state.SignerAddress) | ||
| } | ||
| if q := strings.TrimSpace(req.SignerAddress); q != "" { | ||
| reqAddr := ethcommon.HexToAddress(q) | ||
| if state.SignerAddress != "" && reqAddr != effectiveSigner { | ||
| err := fmt.Errorf("signerAddress does not match payment state") | ||
| respondJsonError(ctx, w, err, http.StatusBadRequest) | ||
| return | ||
| } | ||
| if state.SignerAddress == "" { | ||
| state.SignerAddress = reqAddr.Hex() | ||
| effectiveSigner = reqAddr | ||
| } | ||
| } | ||
| if tk := ls.LivepeerNode.TurnkeyAccount; tk != nil { | ||
| if !ls.LivepeerNode.TurnkeySigningAddressAllowed(effectiveSigner) { | ||
| respondJsonError(ctx, w, fmt.Errorf("signer address not allowed"), http.StatusBadRequest) | ||
| return | ||
| } | ||
| prev := tk.SigningAddress() | ||
| tk.SetSigningAddress(effectiveSigner) | ||
| defer tk.SetSigningAddress(prev) |
There was a problem hiding this comment.
Reject foreign signerAddress values when Turnkey is off.
signState() always uses ls.LivepeerNode.Eth.Sign, and Lines 343-350 only switch keys when TurnkeyAccount exists. If a non-Turnkey node accepts a different req.SignerAddress here, it returns a state that claims signer A but is actually signed by signer B, so the next verifyStateSignature() call fails and the session becomes unusable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/remote_signer.go` around lines 320 - 350, The handler allows accepting
a foreign req.SignerAddress when TurnkeyAccount is nil, causing signState() to
still use the node's Eth.Account().Address and producing invalid state
signatures; change the logic in the request handling around
effectiveSigner/state.SignerAddress (the block that reads req.SignerAddress,
computes effectiveSigner, and later conditionally sets state.SignerAddress) to
reject requests that supply a different signer than the node's own account when
ls.LivepeerNode.TurnkeyAccount == nil by returning a 400 error via
respondJsonError rather than assigning state.SignerAddress; keep the existing
Turnkey path (using TurnkeyAccount, TurnkeySigningAddressAllowed,
SetSigningAddress/ defer restore) unchanged so only non-turnkey nodes enforce
this rejection.
| ls.refreshTurnkeyAddressBook(ctx) | ||
| glog.Info("Turnkey wallet created ", "walletId", wid, "address", addr.Hex()) | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _ = json.NewEncoder(w).Encode(map[string]string{ | ||
| "walletId": wid, | ||
| "address": addr.Hex(), | ||
| }) |
There was a problem hiding this comment.
Propagate address-book refresh failures back to the caller.
Both create endpoints return success before confirming the in-memory allowlist was updated. If refreshTurnkeyAddressBook() fails, the newly returned address can remain unusable for select-address and request authorization until a later refresh or restart.
🔧 Suggested direction
-func (ls *LivepeerServer) refreshTurnkeyAddressBook(ctx context.Context) {
+func (ls *LivepeerServer) refreshTurnkeyAddressBook(ctx context.Context) error {
if ls.TurnkeyAdmin == nil || ls.LivepeerNode == nil || ls.LivepeerNode.TurnkeyOrgID == "" {
- return
+ return nil
}
accts, err := eth.ListTurnkeyEthereumAccounts(ls.TurnkeyAdmin, ls.LivepeerNode.TurnkeyOrgID)
if err != nil {
- clog.Errorf(ctx, "refresh turnkey address book: %v", err)
- return
+ return err
}
list := make([]ethcommon.Address, 0, len(accts))
for _, a := range accts {
list = append(list, a.Address)
}
ls.LivepeerNode.ReplaceTurnkeyAddressBook(list)
+ return nil
}Then fail the create request if the refresh returns an error.
Also applies to: 111-113, 147-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/turnkey_http.go` around lines 75 - 81, The create handlers in
turnkey_http.go call ls.refreshTurnkeyAddressBook(ctx) but ignore its error,
returning success even if the allowlist update failed; update each create
endpoint that calls ls.refreshTurnkeyAddressBook to check its returned error,
and if non-nil respond with an error status (e.g.,
http.StatusInternalServerError) and a JSON error body instead of the success
payload; keep Content-Type header logic, log the error via glog (including
context like walletId/address) and return early on error so the caller only
receives a success response when refreshTurnkeyAddressBook succeeds (apply the
same change to every occurrence of ls.refreshTurnkeyAddressBook in this file).
livepeer#3962) In remote signer mode (-remoteSigner), -cliAddr was never defaulted, so the CLI/management webserver started with an empty address and fell back to ":80", which fails with "permission denied" for non-root users and shuts the node down. Default -cliAddr to 127.0.0.1 like the other node types do, using a dedicated RemoteSignerCliPort (3935) so a co-located broadcaster and remote signer don't collide on the same default CLI port. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Slog (used by the live runner registry, trickle, ai/worker, and media) ran at its default Info level with no way to enable Debug, so every slog.Debug line was dropped regardless of -v. Enable slog Debug at -v 6 (the documented max verbosity) so the existing flag also controls slog output. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…blic address (livepeer#3959) /generate-live-payment signs payments from the node's on-chain deposit, and has no auth when -remoteSignerWebhookUrl is unset (authLivePayment returns 200). A publicly-reachable signer with no webhook can be made to sign tickets for any caller. Refuse to start when -httpAddr is public and no webhook is set, unless the operator opts in with the new -remoteSignerAllowNoAuth flag; warn whenever no webhook is set. Mirrors the broadcaster HTTP-ingest guard. Behavior change: a remote signer on a non-loopback -httpAddr without a webhook now refuses to start until -remoteSignerWebhookUrl or -remoteSignerAllowNoAuth is set. Loopback and webhook setups are unaffected. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Paulius <paulius@Pauliuss-Mini.localdomain>
Builds are extremely flaky; this is an attempt to make them more reliable. There are a couple things contributing to this: - The git-core PPA fails. This appears unnecessary anyway - The zlib / fossils download is unreliable. Use the Github mirror - Update LPMS install_ffmpeg.sh which as the zlib mirror update Claude notes below. --- Replace deprecated `apt-key` and `add-apt-repository` usage in the build workflow and Docker image with explicit `signed-by` keyrings under /usr/share/keyrings and source lists in /etc/apt/sources.list.d. `apt-key` is deprecated and slated for removal, and `add-apt-repository` pulls in software-properties-common; the keyring approach is the recommended method and scopes each key to its own repo instead of trusting it globally. Drop the git-core/candidate PPA in build.yaml and install the distro's stock git instead. The build only needs git new enough for actions/checkout to do a real clone (>=2.18) and for `git describe` versioning; Ubuntu 20.04 (2.25.1) and 22.04 (2.34.1) both satisfy this, so the PPA was unnecessary. Harden network fetches with `curl --retry 5 --retry-delay 2 --connect-timeout 15` to reduce flaky-CI failures, and source the pinned zlib 1.2.11 tarball from GitHub release tags instead of zlib.net/fossils, which has had availability issues. No build outputs or pinned versions change (Go 1.21.5, clang-14, zlib 1.2.11 all retained).
Attempts to fix e2e tests that periodically fail. During e2e startup, the devtool helper and a running orchestrator can both submit initializeRound transactions. If the orchestrator wins, the helper's tx may mine with status 0 even though the round is now initialized. After a failed init tx, re-check CurrentRoundInitialized and treat the operation as successful when the desired state was reached. Still return the original error if the round remains uninitialized.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
cmd/livepeer/starter/starter.go (1)
2219-2228: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
parseHeaderMapdoesn't trim whitespace.If a user writes
"h1:v1, h2:v2"(space after the comma, a natural style), the second header key becomes" h2"with a leading space, silently breaking that header when set viareq.Header.Set. This helper is now shared across three security-relevant header fields (RemoteSignerHeaders,RemoteSignerWebhookHeaders,LiveAIHeartbeatHeaders).🔧 Proposed fix
func parseHeaderMap(raw string) map[string]string { headers := make(map[string]string) for _, header := range strings.Split(raw, ",") { parts := strings.SplitN(header, ":", 2) if len(parts) == 2 { - headers[parts[0]] = parts[1] + headers[strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1]) } } return headers }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/livepeer/starter/starter.go` around lines 2219 - 2228, Update parseHeaderMap to trim surrounding whitespace from each header entry and from both the header name and value before storing them. Preserve the existing comma-separated and first-colon parsing behavior so inputs such as "h1:v1, h2:v2" produce clean keys and values for all callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yaml:
- Around line 136-140: Verify both downloaded archives before extraction: in
.github/workflows/build.yaml lines 136-140, add the expected SHA-256 for
zlib-${ZLIB_VERSION}.tar.gz and validate it before tar -xzf; in
docker/Dockerfile line 17, download go1.21.5.linux-${BUILDARCH}.tar.gz to a
file, validate it against the BUILDARCH-specific SHA-256, then extract only
after verification succeeds.
- Around line 134-140: Update the Windows zlib build step’s ZLIB_VERSION from
1.2.11 to 1.2.12 or newer so the existing download, extraction, and build flow
uses a fixed release. Confirm the FFmpeg build continues to compile successfully
against the upgraded zlib version.
In `@cmd/livepeer/starter/starter.go`:
- Around line 476-485: Update the sensitiveFields map used by PrintConfig to
include LiveAIHeartbeatHeaders as a redacted field, alongside
RemoteSignerHeaders and RemoteSignerWebhookHeaders, so its header values are not
printed unmasked.
- Around line 2032-2052: The *cfg.HttpAddr defaulting currently occurs inside
the remote-signer goroutine, after StartMediaServer may read it. Move the
defaultAddr call before launching the goroutines, in the shared RemoteSignerNode
startup flow, and remove the goroutine-local assignment so both servers
consistently use the finalized bind address.
In `@discovery/wh_discovery.go`:
- Around line 159-171: Update the http.Client used in the webhook request flow
to define CheckRedirect, preventing redirects from forwarding webhook headers to
a different origin. Preserve same-origin redirects if supported, and ensure
cross-origin redirect requests are blocked or have sensitive headers such as
X-API-Key removed before following them.
In `@doc/remote-signer.md`:
- Line 226: Update the “Webhook request” heading in the documentation from h4 to
h3 so it correctly nests under the preceding “Payment Authentication” h2 and
preserves the document heading hierarchy.
In `@install_ffmpeg.sh`:
- Line 3: Update the bash invocation in install_ffmpeg.sh to terminate option
parsing with -- and forward the argument safely using "${1-}"; use "$@" only if
the script is intended to pass through all arguments. Preserve the existing
script URL and execution flow.
---
Nitpick comments:
In `@cmd/livepeer/starter/starter.go`:
- Around line 2219-2228: Update parseHeaderMap to trim surrounding whitespace
from each header entry and from both the header name and value before storing
them. Preserve the existing comma-separated and first-colon parsing behavior so
inputs such as "h1:v1, h2:v2" produce clean keys and values for all callers.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f75c6fa0-0b0c-4cd1-804a-1c92af60cc4a
📒 Files selected for processing (16)
.github/workflows/build.yamlCHANGELOG_PENDING.mdREADME.mdcmd/devtool/devtool/devtool_utils.gocmd/livepeer/livepeer.gocmd/livepeer/starter/flags.gocmd/livepeer/starter/starter.gocore/livepeernode.godiscovery/discovery_test.godiscovery/wh_discovery.godoc/remote-signer.mddocker/Dockerfileinstall_ffmpeg.shserver/live_payment.goserver/remote_signer.goserver/remote_signer_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- server/remote_signer_test.go
- cmd/livepeer/starter/flags.go
- server/live_payment.go
- server/remote_signer.go
- core/livepeernode.go
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 7
🧹 Nitpick comments (1)
cmd/livepeer/starter/starter.go (1)
2219-2228: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
parseHeaderMapdoesn't trim whitespace.If a user writes
"h1:v1, h2:v2"(space after the comma, a natural style), the second header key becomes" h2"with a leading space, silently breaking that header when set viareq.Header.Set. This helper is now shared across three security-relevant header fields (RemoteSignerHeaders,RemoteSignerWebhookHeaders,LiveAIHeartbeatHeaders).🔧 Proposed fix
func parseHeaderMap(raw string) map[string]string { headers := make(map[string]string) for _, header := range strings.Split(raw, ",") { parts := strings.SplitN(header, ":", 2) if len(parts) == 2 { - headers[parts[0]] = parts[1] + headers[strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1]) } } return headers }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/livepeer/starter/starter.go` around lines 2219 - 2228, Update parseHeaderMap to trim surrounding whitespace from each header entry and from both the header name and value before storing them. Preserve the existing comma-separated and first-colon parsing behavior so inputs such as "h1:v1, h2:v2" produce clean keys and values for all callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yaml:
- Around line 136-140: Verify both downloaded archives before extraction: in
.github/workflows/build.yaml lines 136-140, add the expected SHA-256 for
zlib-${ZLIB_VERSION}.tar.gz and validate it before tar -xzf; in
docker/Dockerfile line 17, download go1.21.5.linux-${BUILDARCH}.tar.gz to a
file, validate it against the BUILDARCH-specific SHA-256, then extract only
after verification succeeds.
- Around line 134-140: Update the Windows zlib build step’s ZLIB_VERSION from
1.2.11 to 1.2.12 or newer so the existing download, extraction, and build flow
uses a fixed release. Confirm the FFmpeg build continues to compile successfully
against the upgraded zlib version.
In `@cmd/livepeer/starter/starter.go`:
- Around line 476-485: Update the sensitiveFields map used by PrintConfig to
include LiveAIHeartbeatHeaders as a redacted field, alongside
RemoteSignerHeaders and RemoteSignerWebhookHeaders, so its header values are not
printed unmasked.
- Around line 2032-2052: The *cfg.HttpAddr defaulting currently occurs inside
the remote-signer goroutine, after StartMediaServer may read it. Move the
defaultAddr call before launching the goroutines, in the shared RemoteSignerNode
startup flow, and remove the goroutine-local assignment so both servers
consistently use the finalized bind address.
In `@discovery/wh_discovery.go`:
- Around line 159-171: Update the http.Client used in the webhook request flow
to define CheckRedirect, preventing redirects from forwarding webhook headers to
a different origin. Preserve same-origin redirects if supported, and ensure
cross-origin redirect requests are blocked or have sensitive headers such as
X-API-Key removed before following them.
In `@doc/remote-signer.md`:
- Line 226: Update the “Webhook request” heading in the documentation from h4 to
h3 so it correctly nests under the preceding “Payment Authentication” h2 and
preserves the document heading hierarchy.
In `@install_ffmpeg.sh`:
- Line 3: Update the bash invocation in install_ffmpeg.sh to terminate option
parsing with -- and forward the argument safely using "${1-}"; use "$@" only if
the script is intended to pass through all arguments. Preserve the existing
script URL and execution flow.
---
Nitpick comments:
In `@cmd/livepeer/starter/starter.go`:
- Around line 2219-2228: Update parseHeaderMap to trim surrounding whitespace
from each header entry and from both the header name and value before storing
them. Preserve the existing comma-separated and first-colon parsing behavior so
inputs such as "h1:v1, h2:v2" produce clean keys and values for all callers.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f75c6fa0-0b0c-4cd1-804a-1c92af60cc4a
📒 Files selected for processing (16)
.github/workflows/build.yamlCHANGELOG_PENDING.mdREADME.mdcmd/devtool/devtool/devtool_utils.gocmd/livepeer/livepeer.gocmd/livepeer/starter/flags.gocmd/livepeer/starter/starter.gocore/livepeernode.godiscovery/discovery_test.godiscovery/wh_discovery.godoc/remote-signer.mddocker/Dockerfileinstall_ffmpeg.shserver/live_payment.goserver/remote_signer.goserver/remote_signer_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- server/remote_signer_test.go
- cmd/livepeer/starter/flags.go
- server/live_payment.go
- server/remote_signer.go
- core/livepeernode.go
🛑 Comments failed to post (7)
.github/workflows/build.yaml (2)
134-140: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== .github/workflows/build.yaml relevant lines ==" nl -ba .github/workflows/build.yaml | sed -n '120,155p' echo echo "== zlib references in repository ==" rg -n "ZLIB_VERSION|madler/zlib|zlib-" .github/workflows . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || trueRepository: eliteprox/go-livepeer
Length of output: 249
🌐 Web query:
CVE-2018-25032 zlib affected versions fix release💡 Result:
CVE-2018-25032 is a memory corruption vulnerability in the zlib library that occurs during the deflation (compression) process when processing input containing many distant matches [1][2][3]. Affected Versions The vulnerability affects zlib versions from 1.2.2.2 up to, but not including, 1.2.12 [1][4][3]. The bug was introduced in version 1.2.2.2 with the addition of the Z_FIXED option, which forces the use of fixed Huffman codes [4]. Fix and Release The vulnerability was fixed in zlib version 1.2.12 [1][2][5]. The fix involved modifying the buffer management logic to ensure the pending buffer cannot overwrite the distance symbol table [4]. Users are advised to update to version 1.2.12 or later to remediate this issue [1][3]. Many Linux distributions and other software vendors backported this fix to older versions of the library to address the vulnerability [6][7][8].
Citations:
- 1: https://osv.dev/vulnerability/CVE-2018-25032
- 2: https://nvd.nist.gov/vuln/detail/cve-2018-25032
- 3: https://www.cvedetails.com/cve/CVE-2018-25032/
- 4: madler/zlib#603
- 5: https://www.tenable.com/cve/CVE-2018-25032
- 6: madler/zlib#605
- 7: https://ubuntu.com/security/CVE-2018-25032
- 8: https://cve.report/CVE-2018-25032.pdf
🌐 Web query:
zlib 1.2.11 release notes CVE-2018-25032 1.2.12 1.3.2 current release💡 Result:
As of July 14, 2026, the current version of zlib is 1.3.2, which was released on February 17, 2026 [1][2]. Regarding the specific versions and vulnerability referenced: CVE-2018-25032: This vulnerability involved a memory corruption issue when deflating (compressing) data if the input contained many distant matches [3][4]. It affected zlib version 1.2.11 and earlier versions [3][4]. The issue was addressed and fixed in zlib version 1.2.12, released on March 27, 2022 [5][6]. For context on the versions mentioned: - zlib 1.2.11: The version affected by CVE-2018-25032 [7][3]. - zlib 1.2.12: The release that officially fixed CVE-2018-25032 [4][5]. - zlib 1.3.2: The current release as of this date, which includes further security enhancements and fixes, such as those resulting from a 2026 security audit [2]. The zlib project advises users to always check the official home site (zlib.net) to ensure they are using the latest available version [1][8].
Citations:
- 1: https://zlib.net/
- 2: https://github.com/madler/zlib/releases/tag/v1.3.2
- 3: https://nvd.nist.gov/vuln/detail/cve-2018-25032
- 4: https://osv.dev/vulnerability/CVE-2018-25032
- 5: https://github.com/madler/zlib/blob/v1.2.12/ChangeLog
- 6: https://zlib.net/ChangeLog.txt
- 7: madler/zlib#605
- 8: https://github.com/madler/zlib/blob/develop/README
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== .github/workflows/build.yaml excerpt ==" sed -n '120,155p' .github/workflows/build.yaml | cat -n echo echo "== zlib references in repository ==" rg -n "ZLIB_VERSION|madler/zlib|zlib-" .github/workflows . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || trueRepository: eliteprox/go-livepeer
Length of output: 2179
Upgrade the Windows zlib download to a fixed release.
1.2.11is affected by CVE-2018-25032; bump to1.2.12or newer and confirm FFmpeg still builds against it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yaml around lines 134 - 140, Update the Windows zlib build step’s ZLIB_VERSION from 1.2.11 to 1.2.12 or newer so the existing download, extraction, and build flow uses a fixed release. Confirm the FFmpeg build continues to compile successfully against the upgraded zlib version.Source: MCP tools
136-140: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n## .github/workflows/build.yaml (relevant lines)\n' sed -n '120,155p' .github/workflows/build.yaml printf '\n## docker/Dockerfile (relevant lines)\n' sed -n '1,40p' docker/Dockerfile printf '\n## Search for checksum-related variables/usages\n' rg -n "ZLIB_SHA|SHA256|sha256|go1\.21\.5|madler/zlib|dl\.google\.com/go/go1\.21\.5" .github/workflows/build.yaml docker/DockerfileRepository: eliteprox/go-livepeer
Length of output: 4821
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "## build.yaml around zlib download" nl -ba .github/workflows/build.yaml | sed -n '130,145p' echo echo "## Dockerfile around Go download" nl -ba docker/Dockerfile | sed -n '1,30p' echo echo "## checksum-related references" rg -n "ZLIB_SHA|SHA256|sha256|go1\.21\.5|dl\.google\.com/go|madler/zlib" .github/workflows/build.yaml docker/Dockerfile || trueRepository: eliteprox/go-livepeer
Length of output: 234
Verify downloaded build artifacts before extracting. Both network-fetched archives are trusted as-is, so a compromised mirror or transport path could alter the build output.
.github/workflows/build.yaml#L136-L140: add a SHA-256 forzlib-${ZLIB_VERSION}.tar.gzand check it beforetar -xzf.docker/Dockerfile#L17-L17: downloadgo1.21.5.linux-${BUILDARCH}.tar.gzto a file, verify aBUILDARCH-specific SHA-256, then extract it.📍 Affects 2 files
.github/workflows/build.yaml#L136-L140(this comment)docker/Dockerfile#L17-L17🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yaml around lines 136 - 140, Verify both downloaded archives before extraction: in .github/workflows/build.yaml lines 136-140, add the expected SHA-256 for zlib-${ZLIB_VERSION}.tar.gz and validate it before tar -xzf; in docker/Dockerfile line 17, download go1.21.5.linux-${BUILDARCH}.tar.gz to a file, validate it against the BUILDARCH-specific SHA-256, then extract only after verification succeeds.cmd/livepeer/starter/starter.go (2)
476-485: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
LiveAIHeartbeatHeadersshould also be redacted inPrintConfig.
RemoteSignerHeadersandRemoteSignerWebhookHeaderswere correctly added tosensitiveFields, but the pre-existingLiveAIHeartbeatHeadersfield holds the same class of data (arbitrary headers, e.g.Authorization: Bearer ...) and is printed unredacted byPrintConfig. This diff even touches that field's parsing (line 1927), so it's directly in scope.🔒 Proposed fix
sensitiveFields := map[string]bool{ "EthPassword": true, "OrchSecret": true, "KafkaPassword": true, "MediaMTXApiPassword": true, "LiveAIAuthApiKey": true, "FVfailGsKey": true, "RemoteSignerHeaders": true, "RemoteSignerWebhookHeaders": true, + "LiveAIHeartbeatHeaders": true, }📝 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.sensitiveFields := map[string]bool{ "EthPassword": true, "OrchSecret": true, "KafkaPassword": true, "MediaMTXApiPassword": true, "LiveAIAuthApiKey": true, "FVfailGsKey": true, "RemoteSignerHeaders": true, "RemoteSignerWebhookHeaders": true, "LiveAIHeartbeatHeaders": true, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/livepeer/starter/starter.go` around lines 476 - 485, Update the sensitiveFields map used by PrintConfig to include LiveAIHeartbeatHeaders as a redacted field, alongside RemoteSignerHeaders and RemoteSignerWebhookHeaders, so its header values are not printed unmasked.
2032-2052: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail FILE="cmd/livepeer/starter/starter.go" # Show the relevant region with line numbers. sed -n '1930,2085p' "$FILE" | cat -n echo echo "---- matches for HttpAddr / CliAddr / StartMediaServer / StartCliWebserver ----" rg -n "HttpAddr|CliAddr|StartMediaServer|StartCliWebserver|defaultAddr\\(" "$FILE" echo echo "---- broader node-type address defaulting patterns ----" rg -n "defaultAddr\\(" cmd/livepeer/starter/starter.goRepository: eliteprox/go-livepeer
Length of output: 10199
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the server helpers to understand whether an empty or late-defaulted addr can fail or bind unexpectedly. rg -n "func .*StartMediaServer|func .*StartRemoteSignerServer|func defaultAddr" cmd server -g '!**/*_test.go' echo # Print the implementation slices if they are in the repo tree. for f in $(fd -a "starter.go|server.go" cmd server 2>/dev/null | head -n 20); do echo "==== $f ====" wc -l "$f" doneRepository: eliteprox/go-livepeer
Length of output: 1131
🏁 Script executed:
#!/bin/bash set -euo pipefail # Locate the helper implementations and the address defaulting logic. rg -n "func .*StartMediaServer|func .*StartRemoteSignerServer|func defaultAddr" cmd server -g '!**/*_test.go' echo # Show the relevant file sizes so we can inspect targeted slices next. for f in $(git ls-files 'cmd/**' 'server/**' | rg 'starter\.go$|server\.go$'); do echo "==== $f ====" wc -l "$f" doneRepository: eliteprox/go-livepeer
Length of output: 941
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the relevant server bind behavior and the defaulting helper. sed -n '227,340p' server/mediaserver.go | cat -n echo sed -n '102,180p' server/remote_signer.go | cat -n echo sed -n '2327,2365p' cmd/livepeer/starter/starter.go | cat -nRepository: eliteprox/go-livepeer
Length of output: 8962
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '227,340p' server/mediaserver.go | cat -n echo sed -n '102,180p' server/remote_signer.go | cat -n echo sed -n '2327,2365p' cmd/livepeer/starter/starter.go | cat -nRepository: eliteprox/go-livepeer
Length of output: 8962
🏁 Script executed:
#!/bin/bash set -euo pipefail # Also inspect the exact remote signer block and adjacent launch points for sequencing context. sed -n '1950,2060p' cmd/livepeer/starter/starter.go | cat -nRepository: eliteprox/go-livepeer
Length of output: 4931
Default
*cfg.HttpAddrbefore launching the goroutines
RemoteSignerNodesets*cfg.HttpAddrinside a background goroutine afterStartMediaServer(...)has already been started and may still read the same value. Move the defaulting into the main flow so both servers use the same bind address and avoid the race / split-bind behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/livepeer/starter/starter.go` around lines 2032 - 2052, The *cfg.HttpAddr defaulting currently occurs inside the remote-signer goroutine, after StartMediaServer may read it. Move the defaultAddr call before launching the goroutines, in the shared RemoteSignerNode startup flow, and remove the goroutine-local assignment so both servers consistently use the finalized bind address.discovery/wh_discovery.go (1)
159-171: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Map the relevant file first ast-grep outline discovery/wh_discovery.go --view expanded # Read the relevant section with line numbers sed -n '120,220p' discovery/wh_discovery.go | cat -n # Find all uses of the webhook HTTP client/request in this package rg -n "http\.Client|CheckRedirect|NewRequest\(|Do\(req\)|headers|Headers" discovery -SRepository: eliteprox/go-livepeer
Length of output: 5757
🏁 Script executed:
#!/bin/bash set -euo pipefail GOROOT="$(go env GOROOT)" echo "GOROOT=$GOROOT" # Inspect redirect handling in the Go standard library. grep -nE "func .*redirect|Authorization|Cookie|Referer|Header" "$GOROOT/src/net/http/client.go" | sed -n '1,220p' echo echo "---- relevant client.go slice ----" sed -n '650,860p' "$GOROOT/src/net/http/client.go" | cat -n echo echo "---- docs for Client.Do / CheckRedirect ----" go doc net/http.Client.Do | sed -n '1,120p' echo go doc net/http.Client.CheckRedirect | sed -n '1,120p'Repository: eliteprox/go-livepeer
Length of output: 151
🏁 Script executed:
#!/bin/bash set -euo pipefail GOROOT="$(go env GOROOT)" echo "GOROOT=$GOROOT" # Find the redirect-handling helpers and their line numbers. grep -nE "func .*redirect|shouldCopyHeaderOnRedirect|Authorization|Cookie|Referer|copyHeader|CheckRedirect" "$GOROOT/src/net/http/client.go" echo echo "---- redirect-related implementation slice ----" sed -n '720,840p' "$GOROOT/src/net/http/client.go" | cat -nRepository: eliteprox/go-livepeer
Length of output: 8199
Prevent webhook headers from following redirects.
net/httpcopies non-sensitive headers onto redirect requests by default, so a webhook redirect can send values likeX-API-Keyto a different host. SetCheckRedirectto block cross-origin redirects or filter headers per destination.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@discovery/wh_discovery.go` around lines 159 - 171, Update the http.Client used in the webhook request flow to define CheckRedirect, preventing redirects from forwarding webhook headers to a different origin. Preserve same-origin redirects if supported, and ensure cross-origin redirect requests are blocked or have sensitive headers such as X-API-Key removed before following them.doc/remote-signer.md (1)
226-226: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use an h3 heading for
Webhook request.This heading jumps from
## Payment Authenticationdirectly to####, which breaks heading hierarchy and can affect generated navigation and accessibility. Change it to### Webhook request.🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 226-226: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@doc/remote-signer.md` at line 226, Update the “Webhook request” heading in the documentation from h4 to h3 so it correctly nests under the preceding “Payment Authentication” h2 and preserves the document heading hierarchy.Source: Linters/SAST tools
install_ffmpeg.sh (1)
3-3: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
shellcheck -s bash install_ffmpeg.shRepository: eliteprox/go-livepeer
Length of output: 754
🏁 Script executed:
python3 - <<'PY' import subprocess, textwrap, os, shlex, tempfile, sys tests = [ ["bash", "-s", "-x"], ["bash", "-s", "--", "-x"], ["bash", "-s", "foo bar"], ] script = "printf 'ARGC=%s\\n' $#; printf 'ARGS=<%s>\\n' \"$*\"; printf '1=<%s>\\n' \"$1\"\n" for cmd in tests: p = subprocess.run(cmd, input=script, text=True, capture_output=True) print("CMD:", shlex.join(cmd)) print("RC:", p.returncode) print("STDOUT:\n" + p.stdout.rstrip()) print("STDERR:\n" + p.stderr.rstrip()) print("---") PYRepository: eliteprox/go-livepeer
Length of output: 447
Quote the forwarded argument and end option parsing. Bash still treats
-sinputs as options until--, so an unquoted$1can split/glob and a leading-can be consumed as a flag. Usebash -s -- "${1-}"here; if all args should be forwarded, use"$@"instead.🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 3-3: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@install_ffmpeg.sh` at line 3, Update the bash invocation in install_ffmpeg.sh to terminate option parsing with -- and forward the argument safely using "${1-}"; use "$@" only if the script is intended to pass through all arguments. Preserve the existing script URL and execution flow.Sources: MCP tools, Linters/SAST tools
…nd RLP serialization - Updated `SignTx` method to handle dynamic fee transactions. - Introduced `marshalUnsignedEthereumTx` for proper serialization of unsigned transactions. - Refactored test cases to validate signing against keystore and dynamic fee transactions. - Improved mock signer function for better test coverage and accuracy.
4 participants
This update allows for secure Ethereum key management using Turnkey, enhancing the flexibility and security of the remote signer node.
What does this pull request do? Explain your changes. (required)
Specific updates (required)
How did you test each of these updates (required)
Does this pull request close any open issues?
Checklist:
makeruns successfully./test.shpassSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests