rpc: Report -rpcwhitelist denials as JSON-RPC errors #36753

ryanofsky commented at 12:56 PM on November 4, 2026: contributor

This PR adds two JSON-RPC error codes for requests refused by -rpcwhitelist, which until now got an HTTP 403 status with an empty body:

RPC_METHOD_NOT_PERMITTED (-38) is returned for a refused request, with HTTP 200 for JSON-RPC 2.0 requests and 403 for legacy 1.x requests.
RPC_REQUEST_NOT_EXECUTED (-39) is returned for the other requests in a refused batch. A refused batch now gets HTTP 200 and an array with an error for each request, and still runs none of them.

Problem: -rpcwhitelist errors are currently handled differently from other JSON-RPC errors, so clients report them with vague or misleading messages, and for JSON-RPC 2.0 requests the handling does not follow documented behavior. Specifically, requests refused by -rpcwhitelist get HTTP 403 with an empty body, so the refusal shows up as a generic transport error rather than a specific JSON-RPC error. This is especially a problem for JSON-RPC 2.0 requests, which are supposed to report failed method calls only as JSON-RPC errors, not HTTP errors. The empty reply causes a concrete problem when JSON-RPC requests are sent over IPC instead of HTTP: bitcoin-cli -ipcconnect receives only the response body, so a refused call prints a JSON type error instead of a permission error, as described in a private security review. Over HTTP, bitcoin-cli currently stops at the 403 status and prints only "server returned HTTP error 403" instead of the actual error.

Solution: Report whitelist denials like any other failed call: a JSON-RPC error in the body, with the HTTP status the request's version calls for. This brings the behavior in line with the documentation. bitcoin-cli then prints the permission error over both HTTP and IPC, with no change to the IPC interface. Small change to src/httprpc.cpp and src/rpc/protocol.h, with tests in rpc_whitelist.py and interface_ipc_cli.py. Details in the commit messages.

Compatibility: HTTP clients that send "jsonrpc": "2.0" or batches and check for a 403 status need to read the error code instead. Single 1.x requests keep the 403 status, now with an error body.

Open question for review: RPC_METHOD_NOT_PERMITTED could be named RPC_ACCESS_DENIED, and RPC_REQUEST_NOT_EXECUTED is documented as batch-specific but would fit any request refused before it runs.

rpc: Add RPC_METHOD_NOT_PERMITTED error for -rpcwhitelist denials

Return a JSON-RPC error with a new RPC_METHOD_NOT_PERMITTED (-38) code when
-rpcwhitelist refuses a request, with HTTP status 200 for JSON-RPC 2.0 requests
and 403 for legacy 1.x requests. Previously every refused request got an empty
body and HTTP 403, so the status was the only sign of the refusal.

This makes a whitelist denial follow the rule every other failed method call
has followed since bf1a1f16624 (#27101): the error goes in the response body,
and a 2.0 request gets HTTP 200 unless the HTTP request itself is invalid. A
refused 2.0 notification gets HTTP 204 and no body, like other notifications.
JSONErrorReply() asserts this rule, and doc/JSON-RPC-interface.md documents it
for 2.0 requests, so the 403 that refused 2.0 requests got contradicted the
documentation. This change brings the behavior in line with it, and the
documentation needs no change.

The empty 403 reply broke bitcoin-cli in two ways. Over -ipcconnect it gets the
reply through interfaces::Rpc::executeRpc, which carries no HTTP status, so it
printed a JSON type error. Over HTTP it sends 2.0 for every request and stops
at a 403 status without reading the body, so it printed only "server returned
HTTP error 403". Both now print the permission error.

The denial is reported at the point where a failing method reports its error,
so the existing code handles the version differences and notifications, and the
only new whitelist logic is a WhitelistDenial() check. The same requests are
refused and their methods do not run. One difference: a request has to be parsed
before it can be answered in its own version, so a user refused outright by
-rpcwhitelistdefault is no longer turned away before parsing. A malformed request
from that user now gets the usual parse error, and an empty batch gets an empty
reply.

For review:

- HTTP clients that send "jsonrpc": "2.0" and check for a 403 status need to
  read the error code instead. Legacy 1.x clients see the same status as
  before, now with an error body.
- Code name: RPC_METHOD_NOT_PERMITTED could instead be a more general
  RPC_ACCESS_DENIED. Reusing RPC_INVALID_REQUEST or RPC_METHOD_NOT_FOUND would
  conflict with JSONErrorReply(), which maps them to 400 and 404 for 1.x
  requests.
- A batch is still refused as a whole when any of its methods is denied, so the
  permitted requests in it do not run. Running them would change which requests
  execute, not just what is reported. The next commit keeps this and reports
  the refusal per request.

This issue was reported privately as part of a security audit.

This change was written with Claude Opus 5.

e36e35b87c

rpc: Add RPC_REQUEST_NOT_EXECUTED error for refused -rpcwhitelist batches

When -rpcwhitelist refuses a batch, return HTTP 200 with an array holding one
error per request: RPC_METHOD_NOT_PERMITTED for each denied method and a new
RPC_REQUEST_NOT_EXECUTED (-39) code for the other requests, which did not run.
The -39 message names the denied methods too, because batch clients like
bitcoin-cli -getinfo show only the first error. Previously the whole batch got
HTTP 403 and a single error object (an empty body before the previous commit).
The batch is still refused as a whole before any request runs, so a refused
batch has no partial effect.

Batches report every other error per request in an HTTP 200 response, whatever
the JSON-RPC version, and batch clients cannot read a single error object.
bitcoin-cli -getinfo sends a batch, so a refused one reported "Batch must be an
array" from JSONRPCProcessBatchReply over -ipcconnect, and "server returned
HTTP error 403" over HTTP. It now prints the permission error. Every denial in
the batch is reported, not only the first, so one reply names every method the
caller needs to drop.

The authorization pass also no longer fails the whole batch on a malformed
request. It threw for a request that was not an object, and let an absent or
non-string method field throw out of get_str(), which failed the whole batch
with HTTP 400 or 500. Without a whitelist those requests are reported
individually and the rest of the batch still runs. The pass now leaves them to
the loop that executes the batch, and a refused batch reports their parse
errors the same way, so both paths agree. Nothing slips past the check: parsing
such a request fails there before its method would run.

Requests without an id are left out of the reply, because JSON-RPC 2.0 gives
notifications no response, so a refused batch of only notifications gets HTTP
204 and no body, like an executed one.

For review:

- The status of a refused 1.x batch changes from 403 to 200 too, matching how
  1.x batches report every other error.
- RPC_REQUEST_NOT_EXECUTED is documented as batch-specific, but the name would
  also fit any request refused before it runs.

This issue was reported privately as part of a security audit.

This change was written with Claude Opus 5.

3667e19fe9

DrahtBot added the label RPC/REST/ZMQ on Nov 4, 2026

DrahtBot commented at 12:56 PM on November 4, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36753
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36753.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36753#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	pinheadmz, janb84

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

DrahtBot added the label CI failed on Nov 4, 2026

DrahtBot commented at 2:13 PM on November 4, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/41833576128/job/126586635980</sub> <sub>LLM reason (✨ experimental): CI failed because clang-tidy flagged std::move(*denial) in httprpc.cpp:193 as ineffective on a const reference, and warnings are treated as errors.</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

ryanofsky force-pushed on Nov 4, 2026

ryanofsky commented at 7:08 PM on November 4, 2026: contributor

Updated 8cfdc340b779dc9c31f360900d98b8d807192e9b -> 21526b182fa04f557ca3a55650ff643742b8e94d (pr/rpcdeny.1 -> pr/rpcdeny.2, compare) to fix CI: drop a no-op std::move flagged by clang-tidy https://github.com/bitcoin/bitcoin/actions/runs/41833576128/job/126586635980; and pass the IPC socket path explicitly in the new interface_ipc_cli.py whitelist test, since -ipcconnect=auto can't find it when the datadir path is too long https://github.com/bitcoin/bitcoin/actions/runs/41833576128/job/126586636207

pinheadmz commented at 7:20 PM on November 4, 2026: member

concept ACK Narrowly avoiding a merge conflict with #36758 but I think 36758 should be merged first, since this PR (36753) exposes that bug to a slightly wider use case.

DrahtBot removed the label CI failed on Nov 5, 2026

ryanofsky force-pushed on Nov 5, 2026

ryanofsky commented at 1:01 PM on November 5, 2026: contributor

Updated 21526b182fa04f557ca3a55650ff643742b8e94d -> 3667e19fe9b189662e9ca4c7931c40d348eb34e3 (pr/rpcdeny.2 -> pr/rpcdeny.3, compare) to address findings from the Ralph LLM review (https://review.fish.foo/traces/36753-8cfdc340b779dc9c31f360900d98b8d807192e9b-2121-1.html): name the denied methods in RPC_REQUEST_NOT_EXECUTED errors so bitcoin-cli -getinfo shows them, report parse errors for malformed requests in refused batches, test that a refused batch has no effect, and add a release note

fametrano commented at 7:23 AM on November 10, 2026: contributor

My #36642 gives a non-object batch element a null id, so if it lands, the batch loop here can stay as it is.

janb84 asked on #36642 for the same batch replies with or without -rpcwhitelist, and pinheadmz pointed him here. On master, a user without a whitelist gets the previous element's id on that element's error, and this PR extends that reply to whitelisted users. I ran janb84's batches against each PR and their merge (https://gist.github.com/fametrano/2fc9fb4e35b060c3870aaf198668c271/dd9a540a099199d4293f6854f543915a838b0fbb): with both, each user gets the null id. The two merge cleanly (tree 1042df97fa), and either can land first.

in src/httprpc.cpp:146 in 3667e19fe9

 141 | +    for (size_t i{0}; i < valRequest.size(); ++i) {
 142 | +        if (denied[i].empty()) continue;
 143 | +        const std::string& method{valRequest[i].find_value("method").get_str()};
 144 | +        if (std::ranges::find(denied_methods, method) == denied_methods.end()) denied_methods.push_back(method);
 145 | +    }
 146 | +    const std::string not_executed{strprintf("Not executed because the batch includes methods the user is not permitted to call: %s", util::Join(denied_methods, ", "))};

janb84 commented at 3:26 PM on November 10, 2026:

Think this code still has the DOS vector as highlighted by Ralph

janb84 commented at 3:26 PM on November 10, 2026: contributor

Concept ACK 3667e19fe9b189662e9ca4c7931c40d348eb34e3