fix(azuremanaged): select TLS from endpoint scheme - #360
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
There was a problem hiding this comment.
🟡 Changes recommended
The worker stream recovery e2e test now parses ENDPOINT with new URL(...) at module load, which can throw for schemeless inputs and fail the entire suite to load even when it should be skipped.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the Azure-managed package’s endpoint handling so gRPC transport security (TLS vs plaintext) is selected from the endpoint scheme (http/https) independently of authentication, while tightening safeguards around sending token credentials over insecure (HTTP) endpoints.
Changes:
- Normalize/validate endpoints as URLs: schemeless defaults to
https://, reject unsupported schemes, and select TLS strictly fromhttps://. - Require explicit
allowInsecureCredentials(true)before permitting token credentials overhttp://(without allowing it to downgradehttps://). - Add/adjust tests, scripts, examples, and CI to use explicit
http://for emulator scenarios; add a new endpoint security matrix unit test.
File summaries
| File | Description |
|---|---|
| test/e2e-azuremanaged/worker-stream-recovery.spec.ts | Switch default emulator endpoint to http:// and update port parsing logic. |
| test/e2e-azuremanaged/work-item-filters.spec.ts | Update emulator endpoint default to http://. |
| test/e2e-azuremanaged/rewind.spec.ts | Update emulator usage docs and default endpoint to http://. |
| test/e2e-azuremanaged/retry-handler.spec.ts | Update emulator endpoint default to http://. |
| test/e2e-azuremanaged/retry-advanced.spec.ts | Update emulator endpoint default to http://. |
| test/e2e-azuremanaged/restart.spec.ts | Update emulator endpoint default to http://. |
| test/e2e-azuremanaged/query-apis.spec.ts | Update emulator usage docs and default endpoint to http://. |
| test/e2e-azuremanaged/orchestration.spec.ts | Update emulator endpoint default to http://. |
| test/e2e-azuremanaged/orchestration-id-reuse-policy.spec.ts | Update emulator endpoint default to http://. |
| test/e2e-azuremanaged/history.spec.ts | Update emulator endpoint default to http://. |
| test/e2e-azuremanaged/export-history.spec.ts | Update emulator endpoint default to http://. |
| test/e2e-azuremanaged/entity.spec.ts | Update emulator endpoint default to http://. |
| scripts/test-e2e-azuremanaged.sh | Default ENDPOINT to http://localhost:8080. |
| packages/durabletask-js-azuremanaged/test/unit/worker-builder.spec.ts | Update endpoint constant to include http://. |
| packages/durabletask-js-azuremanaged/test/unit/endpoint-security.spec.ts | New security matrix tests for endpoint scheme vs credential behavior. |
| packages/durabletask-js-azuremanaged/src/worker-builder.ts | Preserve allowInsecureCredentials across connectionString(); stop inferring it from “no credential”. |
| packages/durabletask-js-azuremanaged/src/options.ts | Centralize endpoint URL parsing/validation; choose channel creds from scheme; enforce token-over-HTTP opt-in. |
| packages/durabletask-js-azuremanaged/src/client-builder.ts | Preserve allowInsecureCredentials across connectionString(); rely on per-call metadata for auth. |
| packages/durabletask-js-azuremanaged/README.md | Document transport security and the opt-in requirement for token creds over HTTP. |
| packages/durabletask-js-azuremanaged/CHANGELOG.md | Note TLS selection by scheme + explicit opt-in for token creds over HTTP; schemeless defaults to HTTPS. |
| examples/work-item-filters/package.json | Update emulator start script to use http://localhost:8080. |
| examples/work-item-filters/index.ts | Update emulator endpoint default to http://. |
| examples/TESTING.md | Update expected output to show http:// endpoint. |
| examples/entity-orchestration/README.md | Update expected output to show http:// endpoint. |
| examples/entity-orchestration/package.json | Update emulator start script to use http://localhost:8080. |
| examples/entity-orchestration/index.ts | Update local endpoint default to http://localhost:4001. |
| examples/entity-counter/README.md | Update expected output to show http:// endpoint. |
| examples/entity-counter/package.json | Update emulator start script to use http://localhost:8080. |
| examples/entity-counter/index.ts | Update local endpoint default to http://localhost:4001. |
| .github/workflows/dts-e2e-tests.yaml | Update CI ENDPOINT env var to http://localhost:8080. |
Review details
- Files reviewed: 30/30 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
Document that connectionString replaces prior builder options and cover both call orders for HTTP token credentials. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
Describe connectionString as replacing only connection-derived options. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
Clarify that connectionString replaces the entire builder options object so all later option setters must follow it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
There was a problem hiding this comment.
🟡 Changes recommended
Endpoint parsing rejects supported nested URLs, and the recovery test maps omitted HTTP ports inconsistently.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 30/30 changed files
- Comments generated: 5
- Review effort level: Balanced
Replace generalized separator parsing with named string checks around WHATWG URL parsing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
Make the HTTP default-port exception explicit and use URL.host for standard authority formatting. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
Inline the narrow endpoint shape checks before WHATWG URL parsing while preserving transport and authority behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 05f278e1-d059-477c-a2ed-07aa0921f242
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
There was a problem hiding this comment.
🟡 Changes recommended
Schemeless query and fragment inputs remain accepted, while recovery-test normalization mishandles supported outer whitespace.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 30/31 changed files
- Comments generated: 2
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e0af01a5-0dfa-4e71-a660-c4186e65d7e0
Summary
connectionString()replaces Azure-managed connection options only; builder-level state remains configured, while.allowInsecureCredentials(true)must follow.connectionString(...).@types/nodelock entry from 22.19.9 to 22.20.1. npm 11's current ideal-tree resolution selected 22.20.1 while the stale root lock entry remained at 22.19.9.Endpoint compatibility
Schemeless input is treated as an authority (
example.com,example.com:8443), not as a URL path. Explicit HTTP(S) URLs may include paths, queries, and fragments (for example,https://example.com/callback?next=https://other.test). This matches the documented and pre-change contract without adding parser ambiguity.Validation (heads
3c695f8andf38313a)npm ci: passed with Node 24.14.0 / npm 11.9.0.git diff --check: passed.No real Azure validation was performed because no Azure test credentials or resource configuration were available.