common: don't mention the contents of invalid parameters when in non-dev mode.

Shahana decided this was the optimal UX path, though I insisted that we still
report the actual problem directly when in dev mode, as a compromise.

Suggested-by: https://github.com/Amperstrand
Changelog-Changed: JSON-RPC: Do not return the contents of invalid parameters in error messages, refer to logs (use 'check' to get full error messages)
Fixes: https://github.com/ElementsProject/lightning/issues/7338
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2024-06-23 00:35:33 +09:30
parent e0e38c2cd5
commit f33c5188ef
10 changed files with 116 additions and 15 deletions

View File

@ -19,21 +19,6 @@ struct command_result *command_fail(struct command *cmd, enum jsonrpc_errcode co
/* Caller supplies this too: must provide this to reach into cmd */
struct json_filter **command_filter_ptr(struct command *cmd);
/* Convenient wrapper for "paramname: msg: invalid token '.*%s'" */
static inline struct command_result *
command_fail_badparam(struct command *cmd,
const char *paramname,
const char *buffer,
const jsmntok_t *tok,
const char *msg)
{
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"%s: %s: invalid token '%.*s'",
paramname, msg,
json_tok_full_len(tok),
json_tok_full(buffer, tok));
}
/* Do some logging (complaining!) about this command misuse */
void command_log(struct command *cmd, enum log_level level,
const char *fmt, ...)
@ -68,4 +53,36 @@ bool command_check_only(const struct command *cmd);
struct command_result *command_check_done(struct command *cmd)
WARN_UNUSED_RESULT;
/* Convenient wrapper for "paramname: msg: invalid token '.*%s'" */
static inline struct command_result *
command_fail_badparam(struct command *cmd,
const char *paramname,
const char *buffer,
const jsmntok_t *tok,
const char *msg)
{
if (command_dev_apis(cmd)) {
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"%s: %s: invalid token '%.*s'",
paramname, msg,
json_tok_full_len(tok),
json_tok_full(buffer, tok));
}
/* Someone misconfigured LNBITS with "" around the rune, and so the
* user got a message about a bad rune parameter which *contained the
* rune itself*!. LNBITS should probably swallow any JSONRPC2_* error
* itself, but it is quite possibly not the only case where this case
* where this can happen. So we are a little circumspect in this
* case. */
command_log(cmd, LOG_INFORM,
"Invalid parameter %s (%s): token '%.*s'",
paramname, msg,
json_tok_full_len(tok),
json_tok_full(buffer, tok));
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"%s: %s: invalid token (see logs for details)",
paramname, msg);
}
#endif /* LIGHTNING_COMMON_JSON_COMMAND_H */

View File

@ -33,6 +33,11 @@ struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_e
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_fail called!\n"); abort(); }
/* Generated stub for command_log */
void command_log(struct command *cmd UNNEEDED, enum log_level level UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_log called!\n"); abort(); }
/* Generated stub for command_set_usage */
void command_set_usage(struct command *cmd UNNEEDED, const char *usage UNNEEDED)
{ fprintf(stderr, "command_set_usage called!\n"); abort(); }

View File

@ -62,6 +62,11 @@ struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_e
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_fail called!\n"); abort(); }
/* Generated stub for command_log */
void command_log(struct command *cmd UNNEEDED, enum log_level level UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_log called!\n"); abort(); }
/* Generated stub for command_set_usage */
void command_set_usage(struct command *cmd UNNEEDED, const char *usage UNNEEDED)
{ fprintf(stderr, "command_set_usage called!\n"); abort(); }

View File

@ -50,6 +50,9 @@ struct amount_asset amount_sat_to_asset(struct amount_sat *sat UNNEEDED, const u
/* Generated stub for amount_tx_fee */
struct amount_sat amount_tx_fee(u32 fee_per_kw UNNEEDED, size_t weight UNNEEDED)
{ fprintf(stderr, "amount_tx_fee called!\n"); abort(); }
/* Generated stub for command_dev_apis */
bool command_dev_apis(const struct command *cmd UNNEEDED)
{ fprintf(stderr, "command_dev_apis called!\n"); abort(); }
/* Generated stub for command_fail */
struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_errcode code UNNEEDED,
const char *fmt UNNEEDED, ...)
@ -58,6 +61,11 @@ struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_e
/* Generated stub for command_filter_ptr */
struct json_filter **command_filter_ptr(struct command *cmd UNNEEDED)
{ fprintf(stderr, "command_filter_ptr called!\n"); abort(); }
/* Generated stub for command_log */
void command_log(struct command *cmd UNNEEDED, enum log_level level UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_log called!\n"); abort(); }
/* Generated stub for fmt_amount_sat */
char *fmt_amount_sat(const tal_t *ctx UNNEEDED, struct amount_sat sat UNNEEDED)
{ fprintf(stderr, "fmt_amount_sat called!\n"); abort(); }

View File

@ -67,6 +67,11 @@ bool command_dev_apis(const struct command *cmd UNNEEDED)
/* Generated stub for command_filter_ptr */
struct json_filter **command_filter_ptr(struct command *cmd UNNEEDED)
{ fprintf(stderr, "command_filter_ptr called!\n"); abort(); }
/* Generated stub for command_log */
void command_log(struct command *cmd UNNEEDED, enum log_level level UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_log called!\n"); abort(); }
/* Generated stub for fromwire_sciddir_or_pubkey */
void fromwire_sciddir_or_pubkey(const u8 **cursor UNNEEDED, size_t *max UNNEEDED,
struct sciddir_or_pubkey *sciddpk UNNEEDED)

View File

@ -156,6 +156,9 @@ bool command_deprecated_out_ok(struct command *cmd UNNEEDED,
const char *depr_start UNNEEDED,
const char *depr_end UNNEEDED)
{ fprintf(stderr, "command_deprecated_out_ok called!\n"); abort(); }
/* Generated stub for command_dev_apis */
bool command_dev_apis(const struct command *cmd UNNEEDED)
{ fprintf(stderr, "command_dev_apis called!\n"); abort(); }
/* Generated stub for command_fail */
struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_errcode code UNNEEDED,
const char *fmt UNNEEDED, ...)
@ -169,6 +172,11 @@ struct command_result *command_failed(struct command *cmd UNNEEDED,
/* Generated stub for command_its_complicated */
struct command_result *command_its_complicated(const char *why UNNEEDED)
{ fprintf(stderr, "command_its_complicated called!\n"); abort(); }
/* Generated stub for command_log */
void command_log(struct command *cmd UNNEEDED, enum log_level level UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_log called!\n"); abort(); }
/* Generated stub for command_logger */
struct logger *command_logger(struct command *cmd UNNEEDED)
{ fprintf(stderr, "command_logger called!\n"); abort(); }

View File

@ -3,11 +3,19 @@
#include <common/setup.h>
/* AUTOGENERATED MOCKS START */
/* Generated stub for command_dev_apis */
bool command_dev_apis(const struct command *cmd UNNEEDED)
{ fprintf(stderr, "command_dev_apis called!\n"); abort(); }
/* Generated stub for command_fail */
struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_errcode code UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_fail called!\n"); abort(); }
/* Generated stub for command_log */
void command_log(struct command *cmd UNNEEDED, enum log_level level UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_log called!\n"); abort(); }
/* Generated stub for command_param_failed */
struct command_result *command_param_failed(void)

View File

@ -14,11 +14,19 @@ void notify_log(struct lightningd *ld UNNEEDED, const struct log_entry *l UNNEED
{ }
/* AUTOGENERATED MOCKS START */
/* Generated stub for command_dev_apis */
bool command_dev_apis(const struct command *cmd UNNEEDED)
{ fprintf(stderr, "command_dev_apis called!\n"); abort(); }
/* Generated stub for command_fail */
struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_errcode code UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_fail called!\n"); abort(); }
/* Generated stub for command_log */
void command_log(struct command *cmd UNNEEDED, enum log_level level UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_log called!\n"); abort(); }
/* Generated stub for command_param_failed */
struct command_result *command_param_failed(void)

View File

@ -12,6 +12,7 @@ from utils import (
account_balance, scriptpubkey_addr, check_coin_moves, first_scid
)
import copy
import json
import os
import pytest
@ -4215,3 +4216,31 @@ def test_preapprove_use(node_factory, bitcoind):
l2.rpc.keysend(l1.info['id'], 1000)
with pytest.raises(RpcError, match='keysend was declined'):
l2.rpc.check('keysend', destination=l1.info['id'], amount_msat=1000)
def test_badparam_discretion(node_factory):
"""When in non-developer mode, don't return the contents of invalid parameters, but refer to logs"""
l1 = node_factory.get_node()
with pytest.raises(RpcError, match='rune: should be base64 string: invalid token') as err:
l1.rpc.checkrune(rune='THIS IS NOT ACTUALLY A RUNE')
assert err.value.error['message'] == "rune: should be base64 string: invalid token '\"THIS IS NOT ACTUALLY A RUNE\"'"
# We don't bother logging since we returned all the details
assert not l1.daemon.is_in_log('Invalid parameter')
# Now try non-developer mode (needs some other option removal, too)
l1.stop()
assert l1.daemon.early_opts == ['--developer']
l1.daemon.early_opts = []
opts = copy.copy(l1.daemon.opts)
for k in opts.keys():
if k.startswith('dev'):
del l1.daemon.opts[k]
l1.start()
with pytest.raises(RpcError, match=r'rune: should be base64 string: invalid token \(see logs for details\)'):
l1.rpc.checkrune(rune='THIS IS NOT ACTUALLY A RUNE')
l1.daemon.wait_for_log(r"checkrune: Invalid parameter rune \(should be base64 string\): token '\"THIS IS NOT ACTUALLY A RUNE\"'")

View File

@ -135,6 +135,9 @@ bool command_deprecated_out_ok(struct command *cmd UNNEEDED,
const char *depr_start UNNEEDED,
const char *depr_end UNNEEDED)
{ fprintf(stderr, "command_deprecated_out_ok called!\n"); abort(); }
/* Generated stub for command_dev_apis */
bool command_dev_apis(const struct command *cmd UNNEEDED)
{ fprintf(stderr, "command_dev_apis called!\n"); abort(); }
/* Generated stub for command_fail */
struct command_result *command_fail(struct command *cmd UNNEEDED, enum jsonrpc_errcode code UNNEEDED,
const char *fmt UNNEEDED, ...)
@ -145,6 +148,11 @@ struct command_result *command_failed(struct command *cmd UNNEEDED,
struct json_stream *result)
{ fprintf(stderr, "command_failed called!\n"); abort(); }
/* Generated stub for command_log */
void command_log(struct command *cmd UNNEEDED, enum log_level level UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "command_log called!\n"); abort(); }
/* Generated stub for command_param_failed */
struct command_result *command_param_failed(void)