Skip to content

🐛 hetzner: remodel the HTTP-redirect check the API cannot satisfy as written#2821

Open
tas50 wants to merge 2 commits into
mainfrom
tim/hetzner-remediation-review
Open

🐛 hetzner: remodel the HTTP-redirect check the API cannot satisfy as written#2821
tas50 wants to merge 2 commits into
mainfrom
tim/hetzner-remediation-review

Conversation

@tas50

@tas50 tas50 commented Jun 12, 2026

Copy link
Copy Markdown
Member

Part of the series reviewing every remediation in content/ for correctness (#2775, #2780#2820). This PR covers mondoo-hetzner-security.mql.yaml: all 12 checks were reviewed and all 12 needed fixes. Policy version bumped. Verified against the hetzner provider schema and implementation, the Hetzner OpenAPI spec, the hcloud CLI source, and the terraform provider docs.

A check modeled backwards from the API

lb-http-redirected-to-https treated the redirect as a property of the HTTP service. In Hetzner's API, redirect_http belongs to the HTTPS service ("Only available if protocol is https"), and when enabled the load balancer answers port 80 itself. Consequences: the console step pointed at a control that cannot exist, the CLI command was rejected by the API, the terraform example failed to apply — and independently, the runtime mql read http[\"redirect_http\"] while the provider emits the camelCase redirectHttp, an assertion that was never true. All four variants now assert the corrected model (no plain HTTP service, redirect enabled on the HTTPS service), and the remediation and audit follow it.

Firewall deletions that error or miss IPv6

hcloud firewall delete-rule matches the stored rule with deep equality. The four firewall checks' commands passed --source-ips 0.0.0.0/0 alone, which errors with "rule was not found" on the common rule listing both 0.0.0.0/0 and ::/0, and never cleared IPv6-only rules even though the checks flag them. All four now inspect the rule first, reuse its exact values, add scoped replacements before deleting, and the all-ports fix warns that deleting a firewall's only inbound rule default-denies everything to the attached servers.

Other fixes

  • server-image-not-deprecated claimed Hetzner cannot change a server's image in place; the Rebuild tab and hcloud server rebuild do exactly that (with the disk-erase warning now attached), and the migration path now deletes the old server, without which the check keeps failing.
  • cert-not-expired deleted the certificate before repointing load balancers, breaking TLS mid-rotation; now create, repoint, delete, with a note that an expired managed certificate means automatic renewal failed and needs investigating.
  • lb-https-service-has-certificate used add-service against an existing service, a port conflict; now update-service.
  • blocked-IP checks gain in-list comments explaining the blocked flag is controlled solely by Hetzner's abuse team, replacing comments that sat above the checks.
  • The two terraform remediation entries that pointed at attachment forms the checks don't recognize now steer to the recognized inline forms.

Left for maintainers

  • The terraform variants of the private-network and firewall checks don't recognize the provider's hcloud_server_network, hcloud_firewall_attachment, or apply_to attachment forms, so compliant configs using them false-fail.
  • The firewall checks compare ports as exact strings, so ranges containing the flagged port (and unquoted HCL numbers) evade them.
  • The runtime HTTPS-certificate check is effectively constant-pass since the API requires certificates at HTTPS-service creation; the terraform variant is the one catching real misconfigurations.

Validation

  • cnspec policy lint passes (compiles the remodeled runtime and terraform variant mql)
  • validate_remediation_commands.py hetzner: 46 passed, 0 failed
  • YAML parses clean

🤖 Generated with Claude Code

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well-structured remediation improvements with one potential logic issue in the Terraform HCL query

Comment thread content/mondoo-hetzner-security.mql.yaml Outdated
Comment thread content/mondoo-hetzner-security.mql.yaml
Comment thread content/mondoo-hetzner-security.mql.yaml
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Test Results

  1 files  ±0   44 suites  ±0   1m 25s ⏱️ -14s
854 tests ±0  853 ✅ ±0  1 💤 ±0  0 ❌ ±0 
855 runs  ±0  854 ✅ ±0  1 💤 ±0  0 ❌ ±0 

Results for commit 73b4d30. ± Comparison against base commit 5c0d2f3.

♻️ This comment has been updated with latest results.

@mondoo-code-review mondoo-code-review Bot dismissed their stale review June 12, 2026 14:49

Superseded by new review

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well-structured remediation improvements with one potential logic concern in the HTTP redirect MQL query

Comment thread content/mondoo-hetzner-security.mql.yaml Outdated
Comment thread content/mondoo-hetzner-security.mql.yaml Outdated

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well-structured remediation improvements with one potential logic concern in the MQL query refactor

Comment thread content/mondoo-hetzner-security.mql.yaml Outdated
Comment thread content/mondoo-hetzner-security.mql.yaml Outdated
Comment thread content/mondoo-hetzner-security.mql.yaml
@tas50 tas50 force-pushed the tim/hetzner-remediation-review branch from f8303ec to a24ef82 Compare June 21, 2026 04:13

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well-structured remediation improvements with one potential logic concern in the MQL query change

Comment thread content/mondoo-hetzner-security.mql.yaml
Comment thread content/mondoo-hetzner-security.mql.yaml
@tas50 tas50 force-pushed the tim/hetzner-remediation-review branch from a24ef82 to a025c80 Compare June 23, 2026 00:32

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well-structured remediation improvements with one potential MQL query concern

Comment thread content/mondoo-hetzner-security.mql.yaml
Comment thread content/mondoo-hetzner-security.mql.yaml
Comment thread content/mondoo-hetzner-security.mql.yaml
@tas50 tas50 force-pushed the tim/hetzner-remediation-review branch from a025c80 to c45a3e1 Compare June 24, 2026 16:05

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well-structured remediation improvements with one potential MQL query correctness concern

Comment thread content/mondoo-hetzner-security.mql.yaml
Comment thread content/mondoo-hetzner-security.mql.yaml
@tas50 tas50 force-pushed the tim/hetzner-remediation-review branch from c45a3e1 to e072347 Compare June 26, 2026 20:45

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well-structured remediation improvements with one potential logic concern in the MQL query change

Comment thread content/mondoo-hetzner-security.mql.yaml
Comment thread content/mondoo-hetzner-security.mql.yaml
@tas50 tas50 force-pushed the tim/hetzner-remediation-review branch from e072347 to 2513d74 Compare June 26, 2026 21:16

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well-written remediation improvements and a correct remodel of the HTTP-redirect MQL checks; one potential logic concern in the MQL queries.

Comment thread content/mondoo-hetzner-security.mql.yaml
Comment thread content/mondoo-hetzner-security.mql.yaml
tas50 and others added 2 commits June 29, 2026 10:22
…written

- The redirect check was modeled backwards: Hetzner's redirect_http is
  a property of the HTTPS service, so the console step, CLI flag, and
  terraform example were all rejected by the API, and the runtime mql
  read redirect_http where the provider emits redirectHttp, an
  assertion that was never true. All four variants now assert no plain
  HTTP service plus redirect enabled on the HTTPS service, and the
  remediations follow that model
- hcloud firewall delete-rule matches rules with deep equality, so the
  single-CIDR commands errored on rules listing both 0.0.0.0/0 and
  ::/0 and never cleared IPv6 rules; all four firewall fixes now
  describe first, add replacements before deleting, and warn that
  removing a firewall's only inbound rule blocks all traffic
- Hetzner does support in-place rebuilds; the image check's claim
  otherwise is corrected with data-loss warnings, and the migration
  path now deletes the old server the check still counts
- Certificate rotation reordered to create, repoint, then delete; the
  HTTPS-service fix uses update-service instead of a port-conflicting
  add-service; blocked-IP checks gain in-list comments for the
  CLI/terraform methods Hetzner's abuse team alone controls

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iants

Equivalent logic to the disjunctive form but reads as the intent:
evaluate the redirect block only on https services.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@tas50 tas50 force-pushed the tim/hetzner-remediation-review branch from 2513d74 to 73b4d30 Compare June 29, 2026 17:22

@mondoo-code-review mondoo-code-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Well-structured remediation improvements with one potential logic concern in the MQL query changes

Comment on lines +1584 to +1585
hetzner.loadBalancer.services.none(protocol == "http")
hetzner.loadBalancer.services.where(protocol == "https").all(http["redirectHttp"] == true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 warning — The MQL query now uses two separate statements: none(protocol == "http") and .where(protocol == "https").all(...). If a load balancer has zero HTTPS services (e.g., only TCP services), the .all() on an empty collection will vacuously pass, meaning the check succeeds even though there's no redirect configured. Consider whether you need a guard like services.where(protocol == "https").length > 0 or whether vacuous truth is the intended behavior for non-HTTP/HTTPS load balancers.

Comment on lines +1589 to 1593
terraform.resources("hcloud_load_balancer_service").all(arguments['protocol'] != "http")
terraform.resources("hcloud_load_balancer_service").where(arguments['protocol'] == "https").all(
blocks.where(type == "http").all(arguments['redirect_http'] == true)
)
- uid: mondoo-hetzner-security-lb-http-redirected-to-https-terraform-plan

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 warning — Same vacuous-truth concern in the Terraform HCL variant: if no hcloud_load_balancer_service resources have protocol == "https", the .where(...).all(...) passes trivially. This is consistent with the other variants, but verify this is the desired behavior — a project with only non-HTTP services would pass this check silently.

filters: asset.platform == "hetzner-loadbalancer"
mql: |
hetzner.loadBalancer.services.where(protocol == "http").all(http["redirect_http"] == true)
hetzner.loadBalancer.services.none(protocol == "http")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 suggestion — The Hetzner loadbalancer query uses http["redirectHttp"] (camelCase) while the Terraform queries use http['redirect_http'] (snake_case). This is likely correct since they target different APIs (Hetzner API vs Terraform schema), but worth double-checking that the Hetzner API field is indeed redirectHttp and not redirect_http.

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.

1 participant