Skip to content

Commit c8d144c

Browse files
fix(acme): remove stray paren breaking ACME completion task (v1.8.5, Issue #35)
The background order-completion task (complete_pending_acme_orders, 60s cycle) failed on EVERY cycle since v1.8.0 with: [ACME-COMPLETE] Error in completion task: syntax error at or near ")" Root cause: the bounded DNS-01 retry OR-arm added to the atomic order-claim query in v1.8.0 (13b65d9) carried one extra closing parenthesis, making the whole SELECT invalid PostgreSQL. The claim is the task's first statement, so the generic except swallowed it each minute and NO background ACME work ever ran on v1.8.0-v1.8.4: - orders were never claimed for finalize -> download -> save (http-01 too); - advance_dns01_order never ran, so the DNS-01 TXT record was never published - DNS-01 with an automated provider (e.g. Cloudflare) could never validate (exactly the report in Issue #35); - wizard-staged orders never left wizard_staged (same try block); - retry_invalid_dns01 / reconcile_dns01_cleanup never executed; - hourly-created renewal orders could never complete in the background. Fix: drop the stray ')' (one line). Query semantics are unchanged. Why the suite missed it: the unit tests mock asyncpg, so raw SQL never reaches a real parser. Added a regression test that AST-scans the ACME modules' SQL string literals (comments/quoted literals stripped) and fails on unbalanced parentheses - it is red on the pre-fix tree and would have caught the v1.8.0 regression at commit time. Scanned all six ACME modules: this query was the only unbalanced SQL. Verification: full backend suite in docker green (1062 passed, 151 skipped); the fixed query EXPLAINs cleanly on postgres:15; live localtest run shows zero completion-task errors and a seeded pending order is claimed ("[ACME-COMPLETE] Claimed 1 order(s)"). Backend-only, no schema/API/agent changes; fully backward compatible. Reported-by: @tkkost (GitHub Issue #35)
1 parent 64d4266 commit c8d144c

5 files changed

Lines changed: 113 additions & 5 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2415,6 +2415,7 @@ Developed with ❤️ for the HAProxy community
24152415

24162416
## Release Notes
24172417

2418+
- **v1.8.5** (2026-07-03) — **ACME completion-task SQL fix** (Issue #35 follow-up): the background order-completion task (`complete_pending_acme_orders`, runs every 60s) died on **every cycle** with `syntax error at or near ")"` — an extra closing parenthesis introduced in v1.8.0's bounded DNS-01 retry claim query. Because that query is the task's first database call, **no background ACME work ran at all from v1.8.0 through v1.8.4**: orders were never claimed for finalize/download, the DNS-01 TXT record was never published (so DNS-01 with an automated provider such as Cloudflare could never validate), Site Wizard staged orders never left `wizard_staged`, and DNS-01 retry/TXT-cleanup never executed. The stray parenthesis is removed and a regression test now scans all ACME modules' SQL for unbalanced parentheses (the unit suite mocks the database, which is why a raw-SQL syntax error could slip through). One-line backend query fix; no schema, API, or agent changes — fully backward compatible.
24182419
- **v1.8.4** (2026-06-27) — **Agent installer self-kill fix** (Issue #31): the Linux/macOS agent installer could abort during "pre-installation cleanup" (terminal showed `Killing processes matching: haproxy-agent` then `Killed`) when the install script's own filename contained "haproxy-agent". The cleanup killed processes by matching the bare string "haproxy-agent" against full command lines, which also matched the running installer (and a `sudo`/PAM ancestor the self-exclusion did not cover), so the installer terminated itself. Cleanup now targets only the installed agent (the `$INSTALL_DIR/haproxy-agent` binary and the agent service), never the bare string, and the UI now names the downloaded scripts `install-agent-<platform>.sh` / `uninstall-agent-<platform>.sh`. Installer-only change; the running agent and its privilege model (it runs as root for HAProxy reload, config writes, keepalived, and self-upgrade) are unchanged.
24192420
- **v1.8.3** (2026-06-25) — **Agent heartbeat JSON fix** (Issue #31): a self-hosted agent could fail every heartbeat with `HTTP 400 Invalid JSON: Expecting property name enclosed in double quotes` when the system-info block it collects came back empty on an unusual host, leaving a stray comma in the hand-built heartbeat JSON. The agent script now substitutes a valid placeholder when that block is empty so it can no longer emit a stray comma, and the backend heartbeat endpoint now parses valid payloads as-is and, only when a body fails to parse, tolerates that specific malformed pattern (a leading or doubled comma) so an already-deployed agent recovers on its next heartbeat after this build is deployed. Backend + agent-script only; healthy agents of every version are byte-for-byte unaffected.
24202421
- **v1.8.2** (2026-06-25) — **ACME nonce fix** (Issue #35 follow-up): the ACME client now scopes the anti-replay nonce **per certificate authority** so a nonce issued by one CA is never sent to another. This fixes ZeroSSL/Google account registration failing with `malformed: The Replay Nonce could not be base64url-decoded` (the client previously shared one nonce across CAs and only auto-retried on `badNonce`). Account registration now always uses a fresh nonce from the target CA, and the retry covers this case too. Backend-only; HTTP-01 and Let's Encrypt are unaffected.

backend/main.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,6 @@ async def complete_pending_acme_orders():
318318
OR dns01_last_attempt_at < NOW() - (
319319
(CASE COALESCE(dns01_attempts, 0) WHEN 0 THEN 15 WHEN 1 THEN 30 ELSE 60 END)
320320
|| ' minutes')::INTERVAL
321-
)
322321
)
323322
)
324323
)

backend/tests/test_dns01.py

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,111 @@ def test_nonce_scoped_per_directory():
110110
assert got == "NONCE_A" # returns THIS CA's nonce
111111
assert svc._nonce_by_dir.get("https://a.example/dir") is None # consumed (single-use)
112112
assert svc._nonce_by_dir.get("https://b.example/dir") == "NONCE_B" # the other CA is untouched
113+
114+
115+
def _sql_paren_depth(sql: str):
116+
"""Parenthesis depth of a SQL string, counting only OUTSIDE '...' literals (with ''
117+
escapes), `--` line comments and /* */ block comments. Single-pass state machine so a
118+
`--` inside a literal or a `'` inside a comment cannot corrupt the count. Dollar-quoted
119+
strings are out of scope (not used in this codebase). Returns (final_depth, min_depth).
120+
"""
121+
depth = 0
122+
min_depth = 0
123+
state = "normal"
124+
i, n = 0, len(sql)
125+
while i < n:
126+
ch = sql[i]
127+
nxt = sql[i + 1] if i + 1 < n else ""
128+
if state == "normal":
129+
if ch == "'":
130+
state = "string"
131+
elif ch == "-" and nxt == "-":
132+
state = "line_comment"
133+
i += 1
134+
elif ch == "/" and nxt == "*":
135+
state = "block_comment"
136+
i += 1
137+
elif ch == "(":
138+
depth += 1
139+
elif ch == ")":
140+
depth -= 1
141+
min_depth = min(min_depth, depth)
142+
elif state == "string":
143+
if ch == "'":
144+
if nxt == "'":
145+
i += 1 # escaped '' stays inside the literal
146+
else:
147+
state = "normal"
148+
elif state == "line_comment":
149+
if ch == "\n":
150+
state = "normal"
151+
else: # block_comment
152+
if ch == "*" and nxt == "/":
153+
state = "normal"
154+
i += 1
155+
i += 1
156+
return depth, min_depth
157+
158+
159+
def test_acme_sql_parentheses_balanced():
160+
"""Issue #35 v1.8.5: the completion task's order-claim query shipped (v1.8.0-v1.8.4) with an
161+
extra closing parenthesis, so EVERY 60s cycle died with `syntax error at or near ")"` and no
162+
background ACME work (claim/finalize/download, DNS-01 publish, wizard-staged promotion,
163+
retry, TXT cleanup) ever ran. The suite never caught it because the DB layer is mocked and
164+
raw SQL never reaches a real parser. This guard scans the ACME modules' SQL string literals
165+
for unbalanced parentheses.
166+
167+
Guard scope is deliberately conservative to avoid false positives on production changes:
168+
keyword matching is case-sensitive (SQL is uppercase in this codebase; prose in docstrings
169+
is not) and f-string fragments are excluded (they split at `{`, so a fragment may be
170+
legitimately unbalanced).
171+
"""
172+
import ast
173+
import re
174+
175+
backend_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
176+
modules = [
177+
"main.py",
178+
os.path.join("services", "dns01_orchestrator.py"),
179+
os.path.join("services", "acme_service.py"),
180+
os.path.join("services", "letsencrypt_service.py"),
181+
os.path.join("routers", "letsencrypt.py"),
182+
os.path.join("routers", "acme_diagnostics.py"),
183+
]
184+
problems = []
185+
for rel in modules:
186+
with open(os.path.join(backend_dir, rel), encoding="utf-8") as fh:
187+
tree = ast.parse(fh.read())
188+
fstring_parts = {
189+
id(const)
190+
for joined in ast.walk(tree) if isinstance(joined, ast.JoinedStr)
191+
for const in ast.walk(joined) if isinstance(const, ast.Constant)
192+
}
193+
for node in ast.walk(tree):
194+
if not (isinstance(node, ast.Constant) and isinstance(node.value, str)):
195+
continue
196+
if id(node) in fstring_parts:
197+
continue
198+
sql = node.value
199+
if not re.search(r"\b(SELECT|INSERT|UPDATE|DELETE)\b", sql):
200+
continue
201+
if not re.search(r"\b(FROM|INTO|SET|WHERE)\b", sql):
202+
continue
203+
depth, min_depth = _sql_paren_depth(sql)
204+
if depth != 0 or min_depth < 0:
205+
problems.append(f"{rel}:{node.lineno} (paren depth {depth:+d}, min {min_depth})")
206+
assert not problems, f"Unbalanced parentheses in SQL literal(s): {problems}"
207+
208+
209+
def test_sql_paren_depth_scanner():
210+
# The guard's scanner itself: parens in literals/comments must not count; '' escapes and
211+
# block comments handled; an extra ')' is reported via min_depth even if a later '(' would
212+
# re-balance the total.
213+
assert _sql_paren_depth("SELECT (1)") == (0, 0)
214+
assert _sql_paren_depth("SELECT (1))") == (-1, -1) # the v1.8.0 bug shape
215+
assert _sql_paren_depth("SELECT ')' , '((' FROM t") == (0, 0) # literals ignored
216+
assert _sql_paren_depth("SELECT 'it''s ))' FROM t") == (0, 0) # '' escape stays inside
217+
assert _sql_paren_depth("SELECT 1 -- comment ) (\nFROM t") == (0, 0) # line comment ignored
218+
assert _sql_paren_depth("SELECT 1 /* ) */ FROM t") == (0, 0) # block comment ignored
219+
assert _sql_paren_depth("SELECT 'a--b' AND (x=1\n)") == (0, 0) # -- inside literal is data
220+
assert _sql_paren_depth("WHERE x) AND (y") == (0, -1) # net 0 but went negative

frontend/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "haproxy-openmanager-frontend",
3-
"version": "1.8.4",
3+
"version": "1.8.5",
44
"description": "HAProxy Load Balancer Management UI",
55
"license": "AGPL-3.0-or-later",
66
"dependencies": {

version.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "1.8.4",
3-
"releaseName": "Agent installer self-kill fix",
4-
"releaseDate": "2026-06-27"
2+
"version": "1.8.5",
3+
"releaseName": "ACME completion-task SQL fix",
4+
"releaseDate": "2026-07-03"
55
}

0 commit comments

Comments
 (0)