bcd56b8352a6fad05e623b393f4c8ee5a21944aa
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  shadowdaoandClaude Opus 5
|
4f0949d534 |
fix(certs): stop the test suite bricking the container; make the pairing check real
Gate findings on fix/cert-write-safety. One blocker, one latent truncation
path, a false premise in a load-bearing comment, and a set of tests that were
passing without testing anything.
THE BLOCKER - scripts/test-cert-scripts.py bricked the production container
------------------------------------------------------------------------
The openssl-availability tests build a stripped PATH out of SYMLINKS to real
system binaries (cat, grep, mktemp, mv, cp, rm, mkdir, basename, dirname, find,
date, chmod). _cleanup_tmp() then walked the temp tree calling os.chmod(p,
0o600) - and os.chmod FOLLOWS SYMLINKS. Run once as root in the real image,
which is where this file ships (COPY scripts /haproxy/scripts) and where an
operator would most plausibly run it after deploying a cert fix, it stripped
the exec bit off twelve core binaries INCLUDING chmod itself, so it could not
be undone from inside the container:
/haproxy/scripts/cert-publish-lib.sh: line 109: /usr/bin/grep: Permission denied
bash: /usr/bin/chmod: Permission denied
Certificate publishing stayed dead until the container was recreated. It was
invisible on a workstation because an unprivileged chmod of a root-owned file
fails EPERM straight into `except OSError: pass` - which is also why the
advertised "32 tests pass" was only ever true off-container. In the image the
shipped file measured FAILED (failures=22, skipped=1). Cleanup now skips
symlinks; the suites are green in the image and the binaries survive.
S1 - the shell half had no same-filesystem guard
------------------------------------------------
The header claimed a cross-device mv would "FAIL LOUDLY and leave the live pem
alone". GNU mv does the opposite: across filesystems it copies, so it opens and
truncates the DESTINATION and only then discovers it cannot finish - measured
as a 204800-byte partial live.pem, sentinel gone, before mv reported ENOSPC.
cert_publish() now compares stat -Lc %d of the staging and certs dirs before
writing anything, mirroring the st_dev check the Python half already had, and
the comment says what mv actually does. Latent today (both dirs share a device
on all five hosts) but both are env-overridable.
openssl is present - correct the premise, make the check mandatory
------------------------------------------------------------------
Both halves justified a fail-open with "the image does not necessarily install
the openssl CLI". It does: openssl 3.5.6 in the running container, pulled in by
ca-certificates which certbot needs, and generate_self_signed_cert() already
shells to `openssl req` with check=True during setup. The 'unavailable' branch
never fired, so the pairing check has always run - and that, not the stated
reasoning, is what made the fail-open harmless. Structural validation alone is
weak: a bundle of EMPTY pem blocks passes every structural rule and is caught
only by openssl. The check is now mandatory in both halves and a missing binary
is a loud refusal. No `cryptography` fallback: the app runs on
/usr/local/bin/python3 (3.12) where it is not importable - it belongs to
Debian's /usr/bin/python3 - and reaching for that would be a second unverified
premise.
Smaller items
-------------
* cert_bundle_valid() read the file six times; a concurrent swap between two of
them made openssl x509 and openssl pkey judge different files and log a bogus
"private key does not match the certificate" into the monitored error log. It
now reads one snapshot and feeds openssl from it on stdin.
* except OSError -> except (OSError, UnicodeDecodeError): a BINARY-corrupt live
pem made backup_existing_pem() raise out of publish_pem_bundle() entirely, so
the republish that would have healed the host was the one thing that could
not run. The shell half recovers fine.
* stat -c %a on a symlinked live pem reports the LINK's 0777 and produced a
world-writable private key in the crt directory; now stat -Lc.
* The staging reaper's '*.??????' glob matched mktemp names but not the Python
side's '<name>.<random>.tmp', so those leaked forever. Matches both now.
* renew-certificates.sh and sync-certificates.sh exited 0 even when every
domain failed to publish, so "0 updated, 12 failed" looked identical to a
clean run to cron, to host-renew-certificates.sh (which branches on it) and
to monitoring - a host could silently stop publishing renewals until the
certificates expired. They now exit 1 if any domain failed, still after
publishing the ones that worked.
Test-quality
------------
Four TestCertPublishLibrary tests passed with cert-publish-lib.sh DELETED -
they asserted only rc != 0, and `command not found` is 127. All four now assert
the rejection REASON via assert_rejected(), and setUp() fails if the library is
missing. test_missing_openssl_still_rejects... is replaced by
test_empty_pem_blocks_are_rejected, which pins the case that makes the pairing
check necessary.
Also: the staging-containment test used startswith(certs + os.sep), so
cert_staging_dir() returning the crt directory ITSELF - the exact hazard -
still passed; test_successful_renewal_still_publishes built its "renewed" cert
with a no-op .replace() and could not tell a renewal that published nothing;
test_no_temp_file_survives_a_failed_publish failed before the staging dir
existed and asserted [] == []; FIX_ONLY was skipUnless(hasattr(hm,
'publish_pem_bundle')), so renaming that function turned 10 of 17 tests into
skips while the run still printed OK. Each is fixed and each fix is
mutation-proved: the mutation that the old assertion waved through now fails.
File mode is pinned in both suites (it was pinned nowhere), and the rename
failure is injected with a stub mv instead of chmod 0500, which root ignored -
so that test no longer skips itself precisely where it matters.
Verification
------------
IN THE BUILT IMAGE, as root (the acceptance bar):
scripts/test-cert-scripts.py 38 tests, OK, 0 skipped
scripts/test-cert-write-safety.py 22 tests, OK, 0 skipped
scripts/test-config-rollback.py 17 tests, OK (neighbour, unchanged)
Workstation: 38/38 OK for the shell suite; the Python suite needs flask.
Against the pre-fix tree (main): shell 38 failures; python failures=4, errors=1,
skipped=15 - the FIX_ONLY skips are the 14 fix-only tests plus the API guard.
The old python suite against the pre-fix tree measures skipped=10, confirming
the gate's count.
32 mutation checks, all behaving as intended: every fix breaks a test when
reverted, and every rewritten test fails under the mutation its predecessor
passed. py_compile clean, bash -n clean, shellcheck clean, no new pyflakes
warnings (same 4 pre-existing).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
shadowdaoandClaude Opus 5
|
22dab685d0 |
fix(certs): never truncate a live PEM; validate before publishing
Every path that refreshed a combined certificate did
with open(combined_path, 'w') as combined: # TRUNCATES
subprocess.run(['cat', cert, key], stdout=combined) # rc ignored
where combined_path is the bundle HAProxy is currently serving. The live file
was emptied before any source material had been read and the cat status was
never checked, so an unreadable source, a zero-length privkey, a full disk or a
killed container left a truncated or key-less PEM in place. HAProxy loads
/etc/haproxy/certs as a directory, so one unusable file there fails the whole
ssl bind - HTTPS down for every site on the host, and unlike a bad haproxy.cfg
it is not recoverable by config rollback.
The bundle endpoint compounded it: superseded .pem files were unlinked and
their lineages `certbot delete`d before anything had checked the replacement,
destroying both copies of a working certificate. Recovery there means fresh,
rate-limited ACME orders.
Bundles are now assembled in a staging directory beside the crt directory (never
inside it - HAProxy would try to load a temp file), validated there, and swapped
in with os.replace(). Validation is mandatory structural checks in pure Python
plus a best-effort openssl key/leaf pairing check; a missing openssl warns
loudly and does not silently pass. The previous bundle is copied to
/etc/haproxy/cert-backups first. Superseded certs are moved aside rather than
deleted, and `certbot delete` runs only after HAProxy has reloaded onto the
replacement. The same guarantees are implemented for the cron/renewal shell
path in scripts/cert-publish-lib.sh, which also gates the reload on `haproxy -c`.
Reuses write_config_atomically() from the config-rollback fix (extended with
staging_dir/validate) rather than adding a second atomic writer.
Also heals a zero-byte QUIC cluster-secret file, which previously made
get_or_create_cluster_secret() return '' forever.
Tests: scripts/test-cert-write-safety.py (17) and scripts/test-cert-scripts.py
(32) in the existing stdlib-unittest, stub-binary convention. Both bug classes
reproduce against the pre-fix tree via HAPROXY_MANAGER_DIR.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
shadowdaoandClaude
|
bff18d358b |
Remove set -e and database dependency from certificate scripts
HAProxy Manager Build and Push / Build-and-Push (push) Successful in 56s
Improved certificate renewal and sync scripts to be more resilient: - Removed 'set -e' to prevent silent failures when individual domains error - Scripts now continue processing remaining domains even if one fails - Replaced database queries with direct filesystem scanning of /etc/letsencrypt/live/ - Uses 'find' command to discover all domains with Let's Encrypt certificates - More reliable as it works even if database is out of sync Benefits: - No silent failures - errors are logged but don't stop the entire process - Works independently of database state - Simpler and more straightforward - All domains with certificates get processed regardless of database 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
shadowdaoandClaude
|
1d22d789b8 |
Simplify certificate renewal scripts and add certbot cleanup
HAProxy Manager Build and Push / Build-and-Push (push) Successful in 59s
Simplified all certificate renewal scripts to be more straightforward and reliable: - Scripts now just run certbot renew and copy cert+key files to HAProxy format - Removed overly complex retry logic and error handling - Both in-container and host-side scripts work with cron scheduling Added automatic certbot cleanup when domains are removed: - When a domain is deleted via API, certbot certificate is also removed - Prevents renewal errors for domains that no longer exist in HAProxy - Cleans up both HAProxy combined cert and Let's Encrypt certificate Script changes: - renew-certificates.sh: Simplified to 87 lines (from 215) - sync-certificates.sh: Simplified to 79 lines (from 200+) - host-renew-certificates.sh: Simplified to 36 lines (from 40) - All scripts use same pattern: query DB, copy certs, reload HAProxy Python changes: - remove_domain() now calls 'certbot delete' to remove certificates - Prevents orphaned certificates from causing renewal failures 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> |
||
|
shadowdaoandClaude
|
8d732318b4 |
Fix certificate renewal to properly update HAProxy combined certificate files
HAProxy Manager Build and Push / Build-and-Push (push) Successful in 1m4s
After certbot renews certificates, the separate fullchain.pem and privkey.pem files must be combined into a single .pem file for HAProxy. The renewal script was missing this critical step, causing HAProxy to continue using old certificates. Changes: - Add update_combined_certificates() function to renew-certificates.sh - Query database for all SSL-enabled domains - Combine Let's Encrypt cert + key files using cat (matches haproxy_manager.py pattern) - Always update combined certs after renewal, even if certbot says no renewal needed - Add new sync-certificates.sh script for syncing all existing certificates - Smart update detection in sync script (only updates when source is newer) This ensures HAProxy always gets properly formatted certificate files after renewal. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> |