Skip to content

fix: harden public RPC endpoints against replay, leaks and unbounded queries - #538

Open
KSlashh wants to merge 1 commit into
devfrom
audit/29-1-rpc-security
Open

KSlashh wants to merge 1 commit into
devfrom
audit/29-1-rpc-security

Conversation

@KSlashh

@KSlashh KSlashh commented Nov 12, 2026

Copy link
Copy Markdown
Collaborator

Audit items L39, L40, L41, L43, L47, L67, I01. Open against dev.

Replay after restart (L39). Signed requests are valid for ±300 seconds and their nonces are kept only in memory. After a restart the cache was empty, so a request captured shortly before could be sent again. A signed request stamped before the node started is now rejected. A client that signed just before a restart gets a 401 and signs again.

CORS (L40). A malformed origin or header silently widened the policy to *. CORS_ALLOW_CREDENTIALS=true with an empty or * origin or header list made tower-http panic when the router was built. Both are now startup errors that name the setting. A literal * origin means any origin instead of panicking in AllowOrigin::list. An empty origin list still means any origin, as before.

Proof-desc proxy (L41). The unauthenticated proxy put GOAT_PROOF_BUILD_URL and the upstream error in its response, and retried every request up to 6 times. It now answers proof builder unavailable and logs the details. The RPC HTTP client, used only by this proxy, makes one attempt.

Health endpoint (L67). /v1/health returned the last backend error text, which can contain an RPC URL with an API key. last_error now names the failed operation, e.g. get_block_height failed. The full error is still logged.

Node list (L43). GET /v1/nodes, /v1/nodes/overview and /v1/nodes/{peer_id} wrote this node's timestamp on every request. The heartbeat tick (every 5 minutes) now refreshes it, so the node still counts as online. The status filter ignores case.

Graph list (L47). /v1/graphs now uses the same pagination limits as the other list endpoints: limit 1–100, default 10, offset at most 10000. The query builder no longer emits OFFSET without LIMIT, which SQLite rejects.

Config debug output (I01). GoatInitConfig's Debug printed the private key. It now prints <redacted>.

Tests: a request stamped before the node start is rejected; bad CORS settings are errors and valid ones still build; the health snapshot names the operation and hides the error text; OFFSET without LIMIT gives valid SQL; /v1/graphs rejects limit=1000 and accepts an offset alone; the config's Debug output hides the key. Node, store and client tests, clippy with -D warnings and fmt pass.

The test coverage and validation notes above are from the zkm PR notes. Branch scope and diff formatting were checked locally; builds and tests were not rerun during publication.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant