Skip to content

sync#6928

Merged
neilpang merged 23 commits intomasterfrom
dev
Apr 24, 2026
Merged

sync#6928
neilpang merged 23 commits intomasterfrom
dev

Conversation

@neilpang
Copy link
Copy Markdown
Member

sync

CZECHIA-COM and others added 23 commits March 16, 2026 20:17
* Create dns_czechia.sh

This PR adds a DNS API plugin for CZECHIA.COM / RegZone (ZONER a.s.).
* Add BEST-HOSTING DNS API
* Add DNS hook for subreg.cz
old version had instruction to use bash-only [[ ]] test, remove it and add rules for DNS script writing from #343
Add bHosted.nl DNS API (#6864)
Co-authored-by: Jordan Russell <jordan.russell@sitehost.co.nz>
When the final poll (`_link_cert_retry` at 29) returns, the status
is never checked again. So even a `valid` status goes unnoticed.
It's a pre-test loop after all.

Co-authored-by: Oliver Behnke <oliver.behnke@aei.mpg.de>
* add gname dns acme.sh
- Removed scope exclusion for "standard commit".
  - If 'device-and-networks' is excluded (previous behaviour), a certificate for Panorama (always outside of a template) will not be committed (imported to the config but never applied to Panorama). Therefore, panos.sh was only working for certificates used in templates and applied to devices, but not for the Panorama certificate itself.
  - According to the official documentation and the XML API Browser, there is no 'policy-and-objects' that can be excluded.
  - Although it is not mandatory that the user account is solely dedicated to replace certificates and to perform no other type of operations, it is recommended. If such recommendation is applied, the only changes being committed would be in relation to certificates. Therefore, it should be safe not to exclude any scopes.
- Changed the order for "force commit" from '<commit><partial><force>' (unofficial) to '<commit><force><partial>' (official). Both work, but it is recommended to use what is part of the official documentation and/or XML API Browser.
- Removed unofficial 'policy-and-objects' from commented out code (see above).
- Replaced 'exclude' with 'excluded' from commented out code, as per the official documentation. Both work, but see above.
- Replaced 'acmekeytest' with $_panos_user in the commented out code.

Official documentation: https://docs.paloaltonetworks.com/ngfw/api/pan-os-xml-api-request-types-and-actions/commit
XML API Browser: https://<PANOS HOST>/api
Copilot AI review requested due to automatic review settings April 24, 2026 19:35
@neilpang neilpang merged commit df49dd2 into master Apr 24, 2026
7 checks passed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR syncs several upstream changes into the acme.sh codebase, adding multiple new DNS API providers and adjusting core/auxiliary scripts to support new behaviors and improved robustness.

Changes:

  • Add new DNS API plugins: SiteHost, GNAME, bHosted.nl, and Baidu Cloud BCD DNS.
  • Update existing hooks/scripts (World4You DNS parsing, Synology DSM deploy log message, PAN-OS deploy commit XML).
  • Core updates in acme.sh (make DEFAULT_RENEW env-overridable, pass extendedKeyUsage into CSR creation, and improve link-cert handling + renewAll directory globbing).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
dnsapi/dns_world4you.sh Updates World4You error parsing and record-id detection logic for TXT deletion.
dnsapi/dns_sitehost.sh Adds a new DNS API plugin for SiteHost (NZ).
dnsapi/dns_gname.sh Adds a new DNS API plugin for GNAME.
dnsapi/dns_bhosted.sh Adds a new DNS API plugin for bHosted.nl.
dnsapi/dns_baidu.sh Adds a new DNS API plugin for Baidu Cloud BCD DNS, including request signing.
deploy/synology_dsm.sh Tweaks success-path informational message about HTTP restart behavior.
deploy/panos.sh Adjusts (and comments) PAN-OS partial/force commit XML generation.
acme.sh Makes default renew period configurable, adds EKU support to createCSR, improves cert polling edge case, and adjusts renewAll directory iteration.
.github/copilot-instructions.md Updates internal Copilot guidance table with additional wrapper suggestions.
Comments suppressed due to low confidence (1)

dnsapi/dns_world4you.sh:118

  • Record-id lookup uses grep with regex patterns that include $fqdn and $value unescaped. Since grep treats . and other characters as regex metacharacters, this can match the wrong record (and potentially delete the wrong TXT). Use fixed-string matching (e.g., grep -F) or escape the values before matching. Also handle the case where no matching record is found (empty recordid) by returning success (nothing to remove) instead of issuing a delete with an empty id.
  recordid=$(echo "$form" | grep 'data-records="' | sed 's/.*"\([^"]*\)".*/\1/;s/&quot;/"/g;s/},{/}\n{/g' | grep '"type":"TXT"' | grep "\"name\":\"$fqdn\"" | grep "\"value\":\"$value\"" | sed 's/^.*"id":"\([^"]*\)".*$/\1/')
  _debug recordid "$recordid"

  _resethttp
  export ACME_HTTP_NO_REDIRECTS=1
  body="DeleteDnsRecordForm[id]=$recordid&DeleteDnsRecordForm[uniqueFormIdDP]=$formiddp&DeleteDnsRecordForm[_token]=$form_token"

Comment thread acme.sh
LOCAL_ANY_ADDRESS="0.0.0.0"

DEFAULT_RENEW=30
DEFAULT_RENEW="${DEFAULT_RENEW:-30}"
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEFAULT_RENEW is now overridable via environment, but it’s later used as a numeric value (e.g., assigned into Le_RenewalDays which is compared with -lt). If a user sets DEFAULT_RENEW to a non-integer, it can trigger test/arithmetic errors at runtime. Consider validating that DEFAULT_RENEW is an integer (fallback to 30 if not).

Suggested change
DEFAULT_RENEW="${DEFAULT_RENEW:-30}"
DEFAULT_RENEW="${DEFAULT_RENEW:-30}"
case "$DEFAULT_RENEW" in
"" | *[!0-9]*)
DEFAULT_RENEW=30
;;
esac

Copilot uses AI. Check for mistakes.
Comment on lines +73 to +76
| `[:space:]` | ` ` |
| `[:alnum:]` | `A-Za-z0-9` |
| `[:alpha:]` | `A-Za-z` |
| `[:digit:]` | `0-9` |
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table entries suggesting [:space:] -> and similar mappings for [:alnum:], [:alpha:], [:digit:] are incorrect and could lead to broken regex guidance. POSIX character classes need to be used as [[:space:]], [[:alnum:]], etc., and they are not equivalent to a single literal space or ASCII-only ranges in all locales. Please remove these rows or replace them with correct guidance/examples.

Suggested change
| `[:space:]` | ` ` |
| `[:alnum:]` | `A-Za-z0-9` |
| `[:alpha:]` | `A-Za-z` |
| `[:digit:]` | `0-9` |

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_world4you.sh
return 0
else
msg=$(echo "$res" | grep -A 15 'data-type="danger"' | grep "<h3[^>]*>[^<]" | sed 's/<[^>]*>//g' | sed 's/^\s*//g')
msg=$(echo "$res" | grep -A 20 'alert-notification' | grep 'class="weak-title">[^<]' | sed 's/<[^>]*>//g;s/^\s*//g')
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grep -A is used to extract the error message. BusyBox/portable grep implementations may not support -A, and this repo explicitly avoids grep -A. Also, the sed whitespace trim uses \s (not portable in POSIX sed), so leading whitespace likely won’t be removed. Consider extracting the relevant block without grep -A (e.g., with sed range logic) and trim with [[:space:]] instead.

Suggested change
msg=$(echo "$res" | grep -A 20 'alert-notification' | grep 'class="weak-title">[^<]' | sed 's/<[^>]*>//g;s/^\s*//g')
msg=$(echo "$res" | sed -n '/alert-notification/,$ { /class="weak-title">[^<]/ { s/<[^>]*>//g; s/^[[:space:]]*//; p; q; } }')

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_world4you.sh
return 0
else
msg=$(echo "$res" | grep -A 15 'data-type="danger"' | grep "<h3[^>]*>[^<]" | sed 's/<[^>]*>//g' | sed 's/^\s*//g')
msg=$(echo "$res" | grep -A 20 'alert-notification' | grep 'class="weak-title">[^<]' | sed 's/<[^>]*>//g;s/^\s*//g')
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grep -A is used to extract the error message, which is not portable and is explicitly avoided in this repo. Also, sed uses \s for whitespace trimming, which is not POSIX and likely won’t trim. Please switch to a sed/_egrep_o approach that doesn’t require context grep, and trim with [[:space:]] (or equivalent portable logic).

Suggested change
msg=$(echo "$res" | grep -A 20 'alert-notification' | grep 'class="weak-title">[^<]' | sed 's/<[^>]*>//g;s/^\s*//g')
msg=$(
echo "$res" |
sed -n '/alert-notification/,/class="weak-title">/p' |
_egrep_o 'class="weak-title">[^<]+' |
sed 's/^class="weak-title">//;s/^[[:space:]]*//;s/[[:space:]]*$//'
)

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_bhosted.sh
Comment on lines +69 to +76
_hash="$(_bhosted_cache_hash "$fulldomain" "$txtvalue")"
_rec_id="$(_bhosted_mem_get_id "$_hash")"

if [ -z "$_rec_id" ]; then
_err "No cached bHosted record id found for cleanup."
_err "Please delete TXT manually in bHosted DNS for: ${_bhosted_name}.${_domain}"
return 1
fi
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dns_bhosted_rm can only delete records if the record id was cached from the previous add response. If parsing the id fails (or if the id is missing/expired in domain config), cleanup fails and the script returns an error even though the record may exist. DNS hooks should be able to locate and remove TXT records by name + txtvalue via the provider API (listing records to find ids) instead of relying on cached ids from the add call.

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_bhosted.sh
Comment on lines +41 to +48
# Extract and cache record id in-memory for cleanup in this run
_rec_id="$(_bhosted_extract_id "$response")"
if [ -n "$_rec_id" ]; then
_hash="$(_bhosted_cache_hash "$fulldomain" "$txtvalue")"
_debug "_hash" "$_hash"
_debug "_rec_id" "$_rec_id"
_bhosted_mem_set_id "$_hash" "$_rec_id"
else
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says the record id is cached “in-memory for cleanup in this run”, but _bhosted_mem_set_id persists it via _savedomainconf. This is misleading documentation and can make it harder to understand the lifecycle/cleanup behavior. Either adjust the comment to reflect persistence or change the implementation to truly keep it in-memory.

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_baidu.sh
Comment on lines +320 to +334
_H1="Authorization: $_auth"
_H2="x-bce-date: $_ts"
_H3="x-bce-content-sha256: $_payload_hash"
_H4="Host: $BAIDU_BCD_HOST"
_H5=""

_url="https://${BAIDU_BCD_HOST}${_uri}"
_signed_headers_dbg="$(printf "%s" "$_auth" | cut -d / -f 5)"
_baidu_info "POST ${_uri}"
_baidu_info "signedHeaders: $_signed_headers_dbg"
_baidu_info "payload_sha256: $_payload_hash"
_baidu_debug "baidu_bcd.http.payload" "$(_baidu_dbg_trim "$(_baidu_redact_txt "$_payload")")"
response="$(_post "$_payload" "$_url" "" "POST" "$_content_type")"
_ret="$?"
_baidu_info "ret: $_ret"
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_baidu_bcd_post sets global HTTP header variables _H1.._H4 for Authorization/signing but never unsets/restores them. Since acme.sh _get/_post will include any existing _H1.._H5 on subsequent requests, these auth headers can leak into unrelated HTTP calls after this hook runs. Please save prior _H* values (if any) and unset/restore them after the request (or scope them tightly around the _post call).

Copilot uses AI. Check for mistakes.
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.