feat(proxy): deploy.yml plumbing for redirects_source, deny lists, cert transfer and per-zone DNS providers - #109
Merged
Conversation
Bump MINIMUM_VERSION to the proxy release carrying the dynamic redirect map, deny lists, cert store import/export and per-zone DNS-01 providers (kamal-proxy#86-#90), and regenerate the flag manifest from that tag's binary. The four new deploy flags the manifest gains are wired up in the commits that follow. Refs #108
deploy.yml keys for three of the v1.0.0.2 proxy capabilities, so none of
them needs the run.flags escape hatch:
* proxy/redirects_source {source, interval}: the dynamic redirect map
(--redirects-source/--redirects-interval). Edge disposition - it
answers at the loadbalancer like ssl_domains and canonical_host - and
composes with static redirects (map first, rules on a miss). The
validator mirrors the ssl_domains source shape and refuses an interval
below the proxy's own 10s minimum at config time.
* proxy/deny_ips + proxy/deny_user_agents (--deny-ip/--deny-user-agent),
edge beside allow_ips/rate_limit. IP entries get the allow_ips
validation; UA patterns are shape-checked only (Go RE2 and Ruby Onigmo
disagree at the edges - same reasoning as redirects/rewrites). The
root-healthcheck-path guard, the trusted_proxies no-effect check and
the cannot-identify-clients warning now count deny_ips, since deny
rules key on the same client address.
* proxy/run/acme/dns_provider hash form: zone=provider entries pinning
each zone to the DNS host that serves it, `default` covering the rest,
emitted as repeatable --acme-dns-provider flags with the bare default
last. The string form keeps meaning what it always has.
Refs #108
CLI for the v1.0.0.2 certificate store transfer: export the estate for disaster recovery, import from a Traefik acme.json or restore an exported archive - the Traefik-migration path that must work before the first proxy boot. Both commands target the host that owns TLS: the loadbalancer host when load balancing, else the primary host. The command surface is shared by the proxy and loadbalancer builders (Commands::Proxy::CertTransfer); each supplies its own container, config volume and one-off image, so a loadbalancer sharing a proxy host transfers through the shared kamal-proxy volume. Transport picks the safe path per state: a running container exports through its RPC socket under the proxy's certificate write lock, a stopped one through a one-off offline container. Archives leave via the apps-config bind mount - the one path that is both a host and a container path - and imports stream through stdin into the one-off container, because a bind-mounted source would need host permissions the container user cannot be guaranteed to have, and the store must be written as the image's own user. Import refuses a running container (offline-only in the proxy) except for --verify, which only reads the archive; contradictory flag combinations fail before anything uploads, mirroring kamal-proxy's own flag groups. Refs #108
There was a problem hiding this comment.
4 issues found across 19 files
Confidence score: 2/5
- In
lib/kamal/commands/proxy/cert_transfer.rb, interpolating--resolverinto a single-quotedsh -cpayload allows apostrophes to break quoting and potentially execute arbitrary commands on the target host, which is the highest-risk path here — route this argument throughBase#shell(or equivalent strict escaping) before command assembly. - In
lib/kamal/cli/proxy.rb, the upload/download failure paths can leave certificate material and private-key archives on the remote host, creating a concrete secret-retention risk after partial failures — moveupload!and archive handling underbegin/ensureso remote cleanup always runs. - In
lib/kamal/configuration/validator/proxy.rb, accepting malformed or non-string zone keys can generate unusable--acme-dns-providermappings, causing affected ACME challenges to run without a matching provider and fail unexpectedly — validate each zone key as a non-empty string during config checks.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/kamal/configuration/validator/proxy.rb">
<violation number="1" location="lib/kamal/configuration/validator/proxy.rb:165">
P2: Malformed or non-string zone keys can pass validation and produce unusable `--acme-dns-provider` entries, causing ACME challenges for those zones to use no matching provider. Validating each key as a non-empty string without whitespace or `=` would prevent this silent misconfiguration.</violation>
</file>
<file name="lib/kamal/cli/proxy.rb">
<violation number="1" location="lib/kamal/cli/proxy.rb:527">
P1: Failed archive downloads leave a private-key archive on the remote host; run `remove_certs_archive` in an `ensure` block around the download.</violation>
<violation number="2" location="lib/kamal/cli/proxy.rb:562">
P1: Failed source uploads leave certificate material on the remote host; include `upload!` in the existing cleanup `begin`/`ensure`.</violation>
</file>
<file name="lib/kamal/commands/proxy/cert_transfer.rb">
<violation number="1" location="lib/kamal/commands/proxy/cert_transfer.rb:47">
P1: A `--resolver` containing an apostrophe can break out of the single-quoted `sh -c` payload and execute arbitrary shell commands on the target host; use the existing `Base#shell` quoting helper (or explicitly escape single quotes) when constructing this nested command.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
…0.0.3 The repos moved to the zoolutions org (enterprise Actions minutes), and a workflow's GITHUB_TOKEN can only write packages in its own owner namespace - so the proxy's publish workflow could no longer push to the mhenrixon user package, which is why the v1.0.0.2 image never appeared. The image now publishes as ghcr.io/zoolutions/kamal-proxy (zoolutions/kamal-proxy#98); this points the gem's repository defaults, docs, tests, integration harness, toolkit and the dash CLI image workflow at the new org. MINIMUM_VERSION moves to v1.0.0.3: the v1.0.0.2 tag predates the workflow fix, so its tag-triggered build pushes to the old org and can never publish - v1.0.0.3 is the same Go code cut from dash after the fix. Manifest regenerated for it (flag surface unchanged). Refs #108
mhenrixon
added a commit
to zoolutions/kamal-proxy
that referenced
this pull request
Aug 10, 2026
The repo moved to the zoolutions org for enterprise Actions minutes, and a workflow's GITHUB_TOKEN can only write packages in its own owner namespace - pushing to the mhenrixon user package now fails with "permission_denied: The requested installation does not exist" (which is why v1.0.0.2 never published). Point the publish workflow, the release script and the docs at ghcr.io/zoolutions/kamal-proxy. The dash gem's repository defaults move in lockstep (zoolutions/kamal#109).
There was a problem hiding this comment.
1 issue found across 26 files (changes from recent commits).
Confidence score: 4/5
- In
lib/kamal/configuration/proxy/run.rb, the default proxy image changed toghcr.io/zoolutions/kamal-proxywhiletest/integration/main_test.rbstill assertsghcr.io/mhenrixon/kamal-proxy, creating a config/test mismatch that can either fail CI or leave the rename unverified in integration coverage—update the integration assertion (and any related fixtures) to the new default image path.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/kamal/configuration/proxy/run.rb">
<violation number="1" location="lib/kamal/configuration/proxy/run.rb:98">
P2: The default proxy image was renamed from ghcr.io/mhenrixon/kamal-proxy to ghcr.io/zoolutions/kamal-proxy, but test/integration/main_test.rb (line 49) still asserts on the old `ghcr.io/mhenrixon/kamal-proxy` image path. Since that file is now inconsistent with the renamed default the proxy command produces, the integration assertion will not match the generated `pull`/`run` output (or will silently reference the wrong registry). Please update main_test.rb to `ghcr.io/zoolutions/kamal-proxy` so the rename is applied consistently repo-wide.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Without set -e a failed proxy-image seed (pull/tag/push into registry:4443) exits 0 - the script ends with rm -f - and the miss surfaces much later, deep inside an unrelated deploy, as "manifest unknown". Exactly the failure-at-a-distance the seeding comment warns about; now the setup step fails on the line that broke.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 7 unresolved issues from previous reviews.
Re-trigger cubic
Review findings from PR #109 (cubic): * import: an apostrophe in --resolver could end the single-quoted sh -c payload and run arbitrary commands on the target host - build the payload with Base#shell, which escapes embedded quotes. * export: take the deploy lock (a concurrent deploy could reboot the proxy mid-export and tear the offline read), and remove the archive in an ensure so a failed download does not leave private keys on the host. * import: move upload! inside the ensure's reach so a failed or partial upload cannot leave certificate material behind either. * acme: a non-string, blank or whitespace zone key would emit an --acme-dns-provider entry no zone ever matches - reject it at config time. * finish the ghcr org sweep: test/integration/main_test.rb and the testing-rules examples spell the image with regex-escaped dots, which the plain-string replace could not match.
The harness's docker_compose captures stdout and discards the stderred block, so a docker daemon error in the seed (they all print to stderr) left three CI runs failing blind after "gems installed". exec 2>&1 puts everything setup.sh does onto the one stream the failure message carries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
deploy.yml and CLI plumbing for the four proxy capabilities from kamal-proxy#86–#90, released as kamal-proxy v1.0.0.3, so none of them needs the
run.flagsescape hatch:proxy.redirects_source: {source, interval}→--redirects-source/--redirects-interval. Edge disposition (answers at the loadbalancer, likessl_domains); composes with staticredirects:(map first, static rules on a miss). Validator mirrors thessl_domainssource shape and refuses an interval below the proxy's 10s minimum at config time.proxy.deny_ips/proxy.deny_user_agents→--deny-ip/--deny-user-agent, edge besideallow_ips/rate_limit. IP entries get theallow_ipsvalidation; the cannot-identify-clients warning, the root-healthcheck-path guard and thetrusted_proxiesno-effect check now countdeny_ipstoo.kamal proxy export_certs LOCAL_PATHandkamal proxy import_certs --traefik-acme|--archive … [--resolver|--force|--verify], targeting the TLS-owning host (loadbalancer host when load balancing, else the primary host). Import works before the first proxy boot (one-off container creates the config volume) — the Traefik-migration path.proxy.run.acme.dns_providergains a hash form (zone: provider+default:), emitted as repeatable--acme-dns-providerflags; the string form is unchanged.MINIMUM_VERSIONmoves tov1.0.0.3and the flag manifest was regenerated from that tag's binary — the coverage canary confirms the four new deploy flags are the only additions, all now emitted.Closes #108
Test plan
bundle exec rubocop --parallelcleanbin/test(integration) — runnable onceghcr.io/zoolutions/kamal-proxy:v1.0.0.3is published and publicImage org move (why v1.0.0.3, not v1.0.0.2)
The repos moved to the zoolutions enterprise org, and a workflow's
GITHUB_TOKENcan only write packages in its own owner namespace — so the proxy's publish workflow could no longer push to themhenrixonuser package (permission_denied: The requested installation does not exist), and the v1.0.0.2 image never appeared. zoolutions/kamal-proxy#98 pointed the publish atghcr.io/zoolutions/kamal-proxy; since the v1.0.0.2 tag predates that fix (its workflow forever pushes to the old org), v1.0.0.3 was cut fromdashwith identical Go code. This PR moves the gem in lockstep:Proxy::Run#repository/Proxy::Boot#repository_namedefaults, docs, tests, integration harness, toolkit docs, and the dash CLI image workflow (ghcr.io/zoolutions/dash) all point at the new org.Post-publish checklist: make the new ghcr packages (
zoolutions/kamal-proxy, laterzoolutions/dash) public — deploys and integration tests pull anonymously. Existing deploys pullingghcr.io/mhenrixon/kamal-proxykeep working until rebooted onto the new default.Deviations & judgment calls
Validator::Proxy#validate_path_rules!: Go RE2 and Ruby Onigmo differ at the edges; refusing a pattern kamal-proxy would accept is worse than finding out at deploy time). Followed the codebase precedent:deny_user_agentsentries are shape-checked (non-empty strings), not compiled.acme.credentials". The gem has no provider→env-var map (credentials are opaque secret names), so this is unimplementable without hardcoding lego's per-provider env matrix — new drift surface. Provider names are validated; credentials are not cross-checked.import_certsrefuses a running container (the proxy's import is offline-only; its own runbook is stop → import → start), except--verify, which only reads the archive. Export prefersdocker execinto the running container (RPC path, taken under the certificate write lock) and falls back to a one-off offline container when stopped.sh -c 'cat > /tmp/…') instead of bind-mounting it: a bind mount would need host file permissions the container user cannot be guaranteed to have, and running the importer as root would leave a store the proxy user cannot read. Export leaves through the apps-config bind mount — the one path that is both a host path and a container path.import_certs/export_certs(Thor underscore convention, likeboot_config), not the issue'simport-certs.ensure_rate_limit_can_identify_clientstodeny_ips(deny_user_agentsnever keys on the client address, so it is excluded). In-path sibling extensions the issue didn't ask for: the healthcheck-path guard andtrusted_proxiesno-effect check now countdeny_ips; the docs'run.versionexample bumped to v1.0.0.3 so it no longer pins below the new default.docker loginfirst — the image may not exist on the host yet (the before-first-LB-boot migration case).acme.dns_providerswitched to the hash form (the example-driven validator needs a Hash example for the override hook to fire); the string form is documented in prose and covered byvalidate_key_override!.