Summary
The DNS API test suite (le_test_dnsapi in letest.sh) only exercises the happy
path with a valid API key. There is no test that verifies a plugin correctly
fails when given a bad/invalid API key. As a result, plugins that silently report
success on an error response are not caught by CI.
Why this matters
If a DNS plugin returns success on a failed API call, acme.sh will confirm that a
DNS record was created when it actually was not. This leads to confusing,
hard-to-debug failures later in the flow (challenge validation fails with no clear
cause) and erodes trust in the tool. Errors should surface at the point where they
occur, not silently downstream.
Concrete evidence
Real example from dns_infomaniak.sh. The success check was:
if [ ! "$(echo "$response" | _contains '"result":"success"')" ]; then
_info "Record added"
return 0
fi
This was effectively constant-true:
- _contains ignores stdin, so the piped $response was discarded.
- The pattern '"result":"success"' was passed as $1 (haystack), leaving $2
(needle) empty, so it ran grep -- "", which always matches.
- That grep output is redirected to /dev/null, so the command substitution always
captured "", making [ ! "" ] always true.
Result: on an invalid token (response {"result":"error",...}) the plugin still
reported "Record added" and returned 0. The happy-path test passed regardless, so
the bug was invisible to CI. The fix simply uses _contains directly by exit code:
if _contains "$response" '"result":"success"'; then.
Fix PR: acmesh-official/acme.sh#7048
Proposal
Add a negative test path. Two complementary options:
- Offline / mocked (preferred for determinism): stub _post/_get to return a
canned {"result":"error"} body and assert _add / _rm return
non-zero; also assert a {"result":"success"} body returns zero. No network, no
secrets, and not subject to DNS-propagation flakiness.
- Live bad-key smoke test: set the provider's token env var to an obviously
invalid value (e.g. test123) and assert _add returns non-zero.
Required harness change
The current assertion helpers (_assertcmd, etc.) only assert success (exit 0).
There is no "assert this command fails" helper. This proposal needs:
- A new helper, e.g. _assertfail , that passes when the command returns
non-zero.
- A new case, e.g. le_test_dnsapi_badkey, driven by TEST_DNS so it is
provider-agnostic.
Caveats / open questions
- Live bad-key tests make real (failing) API calls — consider provider rate limits.
- How to inject a bad value without clobbering the real TokenValue* secret (e.g. a
dedicated case that overrides the token env var in a subshell).
- Whether to gate the live variant behind an opt-in env flag, and keep the mocked
variant always-on.
Summary
The DNS API test suite (
le_test_dnsapiinletest.sh) only exercises the happypath with a valid API key. There is no test that verifies a plugin correctly
fails when given a bad/invalid API key. As a result, plugins that silently report
success on an error response are not caught by CI.
Why this matters
If a DNS plugin returns success on a failed API call, acme.sh will confirm that a
DNS record was created when it actually was not. This leads to confusing,
hard-to-debug failures later in the flow (challenge validation fails with no clear
cause) and erodes trust in the tool. Errors should surface at the point where they
occur, not silently downstream.
Concrete evidence
Real example from
dns_infomaniak.sh. The success check was:This was effectively constant-true:
(needle) empty, so it ran grep -- "", which always matches.
captured "", making [ ! "" ] always true.
Result: on an invalid token (response {"result":"error",...}) the plugin still
reported "Record added" and returned 0. The happy-path test passed regardless, so
the bug was invisible to CI. The fix simply uses _contains directly by exit code:
if _contains "$response" '"result":"success"'; then.Fix PR: acmesh-official/acme.sh#7048
Proposal
Add a negative test path. Two complementary options:
canned {"result":"error"} body and assert _add / _rm return
non-zero; also assert a {"result":"success"} body returns zero. No network, no
secrets, and not subject to DNS-propagation flakiness.
invalid value (e.g. test123) and assert _add returns non-zero.
Required harness change
The current assertion helpers (_assertcmd, etc.) only assert success (exit 0).
There is no "assert this command fails" helper. This proposal needs:
non-zero.
provider-agnostic.
Caveats / open questions
dedicated case that overrides the token env var in a subshell).
variant always-on.