Skip to content

feat: wire --insecure flag to HTTPS cert verification#12

Merged
paveg merged 2 commits into
mainfrom
feat/insecure-tls-verify
Apr 10, 2026
Merged

feat: wire --insecure flag to HTTPS cert verification#12
paveg merged 2 commits into
mainfrom
feat/insecure-tls-verify

Conversation

@paveg
Copy link
Copy Markdown
Owner

@paveg paveg commented Apr 10, 2026

Summary

  • Bump `moonbitlang/async` 0.16.0 → 0.17.1 and `mizchi/tui` 0.9.0 → 0.9.1 to gain the new `verify~` parameter on `@http.Client::new` (released upstream 2026-04-10).
  • Pass `verify~=!config.insecure` at all three `@http.Client::new` sites in `src/lib/worker/worker.mbt` (initial connect, `disable_keepalive` reconnect, post-error reconnect). The `--insecure` / `-k` CLI flag was previously plumbed through `Config` but silently no-op'd at the client boundary — it now actually disables TLS certificate validation.
  • Adapt to async 0.17.0 breaking changes in `src/lib/coordinator/coordinator_test.mbt`: `@http.Server::new` became `async` (so `start_server` is now `async fn`), and `.addr()` is deprecated in favor of the `.addr` field.
  • Remove the `--insecure` caveat from the "Known Constraints" section of `CLAUDE.md`.

Why now

`moonbitlang/async` v0.17.1 (2026-04-10) finally shipped the `verify?: Bool = true` parameter that resolves moonbitlang/async#329. `mizchi/tui` v0.9.1 (same day) relaxed its transitive pin via mizchi/tui.mbt#9 so molt can actually consume it — MoonBit's resolver doesn't allow two versions of the same module in the graph, and tui was previously hard-pinning async at 0.16.6.

Test plan

  • `moon check --target native` — clean (0 errors, 0 warnings)
  • `moon fmt` — no changes
  • `moon test --target native` — 250 passed, 0 failed
  • `moon build --target native --release` — release binary 2.5M (within CLAUDE.md's ~2.4MB guideline)
  • Manual: `molt -k https://self-signed.badssl.com -n 1` succeeds (would fail pre-change with TLS verification error)
  • Manual: `molt https://self-signed.badssl.com -n 1` (without `-k`) still fails with TLS verification error, confirming the flag is the thing that toggles behavior

Notes for reviewers

  • No TDD-style unit test for the verify flag itself: `@http.Client` doesn't expose an introspection API for the `verify` state, and a proper self-signed-cert integration test would need a TLS-capable test server which isn't readily available in the current test harness. The actual TLS bypass is covered by manual verification (see test plan). Open to suggestions if reviewers want a stronger automated check.
  • `@http.Client` in async 0.17.1 also enables automatic gzip request/response handling when `Accept-Encoding` isn't explicitly set. This may shift `response_size_bytes` measurements for targets that gzip responses. Not addressed in this PR — worth a follow-up issue to decide whether to force `Accept-Encoding: identity` or document the behavior change.
  • The project rule in `.claude/rules/moonbit.md` says "Boolean negation: `not(x)` (not `!x`)", but the current MoonBit compiler emits a deprecation warning for `not()` and recommends `!`. There are zero existing `not(` usages in `src/`, so this PR uses `!config.insecure`. Rule file update is out of scope here but probably worth following up on.

🤖 Generated with Claude Code

Bump moonbitlang/async 0.16.0 → 0.17.1 (ships the verify~ parameter
on @http.Client::new) and mizchi/tui 0.9.0 → 0.9.1 (which relaxed its
async pin to allow 0.17.x). Pass verify~=!config.insecure at all three
@http.Client::new sites in worker.mbt so the existing --insecure CLI
flag actually reaches the TLS layer instead of silently no-op'ing.

Also adapt to async 0.17.0 breaking changes in coordinator_test.mbt:
@http.Server::new became async (start_server now async fn), and the
.addr() getter is deprecated in favor of the .addr field.

Verified: moon check clean, moon fmt clean, moon test --target native
250/250 passed, release binary 2.5M.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 10, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.48%. Comparing base (843f4a4) to head (fc9c881).

Files with missing lines Patch % Lines
src/cmd/main/main.mbt 0.00% 4 Missing ⚠️
src/lib/worker/worker.mbt 33.33% 2 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #12      +/-   ##
==========================================
- Coverage   62.73%   62.48%   -0.26%     
==========================================
  Files          16       16              
  Lines         730      733       +3     
==========================================
  Hits          458      458              
- Misses        272      275       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Previously --debug used @http.get(uri) — a module-level helper that
doesn't accept verify~, so -k silently failed even after the main
load-test path honored it. Route debug through Client::new with
verify~=!config.insecure so both code paths behave consistently.

- Expose worker.parse_url as pub for reuse from main.mbt
- Import moonbitlang/async/io aliased as @async_io to avoid name
  collision with the existing mizchi/tui/io @io alias
- Read body via (client : &@async_io.Reader).read_all() since
  Client::get returns only Response (not (Response, &@io.Data) like
  the module-level helper)

Manual verification against https://self-signed.badssl.com:
- molt --debug https://...       → TlsError (expected)
- molt --debug -k https://...    → 200 OK + body (expected)
- molt -n 1 https://...          → 50 failed with TlsError
- molt -n 1 -k https://...       → 55 succeeded with 200

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@paveg
Copy link
Copy Markdown
Owner Author

paveg commented Apr 10, 2026

Follow-up: debug mode also wired to `--insecure`

Manual verification against `https://self-signed.badssl.com\` surfaced a gap: `--debug` used the module-level `@http.get(uri)` helper, which doesn't accept `verify~`. So `molt --debug -k https://self-signed.badssl.com\` still failed with `TlsError` even after the worker path was fixed.

Pushed `fc9c881` to route debug mode through `@http.Client::new(base_url, verify~=!config.insecure)` as well. Both paths now behave consistently.

Verification matrix

Mode `-k` off `-k` on
`--debug` ❌ `TlsError` ✅ `200 OK` + body
load test (`-n 1`) ❌ 50 × `TlsError` ✅ 55 × `200`

Implementation notes

  • `worker.parse_url` is now `pub` so `main.mbt` can reuse the base/path split without duplicating logic.
  • `moonbitlang/async/io` is imported as `@async_io` in `src/cmd/main/moon.pkg` to avoid a name collision with the existing `mizchi/tui/io` `@io` alias.
  • Body reading uses `(client : &@async_io.Reader).read_all()` because `Client::get` returns only `Response` (unlike the module-level `@http.get` which returns `(Response, &@io.Data)`).

Test plan (updated)

@paveg paveg merged commit 69d81cb into main Apr 10, 2026
4 checks passed
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.

feat: pass verify parameter through HTTP Client to TLS layer

2 participants