feat: support allow-insecure HTTP dependencies by arika0093 · Pull Request #700 · microsoft/apm

arika0093 · 2026-04-14T08:18:41Z

Description

This PR adds explicit support for HTTP (insecure) APM dependencies behind an opt-in flow, and makes lockfile replays preserve that HTTP source information.

When adding a package, you can explicitly allow HTTP dependencies by providing the --allow-insecure flag.

$ apm install --allow-insecure http://my-server.example.com/owner/repo

# if not provided, it will error like this:
# [*] Validating 1 package...
# [x] http://my-server.example.com/owner/repo -- 'http://my-server.example.com/owner/repo' uses HTTP (insecure). Pass '--allow-insecure' or run 'apm config set allow-insecure true' to allow HTTP dependencies.
# All packages failed validation. Nothing to install.

When restoring packages, you also need to provide the --allow-insecure flag unless the global config is enabled. If there are HTTP URLs in the dependency chain, it will error without --allow-insecure or apm config set allow-insecure true.

$ apm install --allow-insecure

# if not provided, it will error like this:
# [>] Installing dependencies from apm.yml...
# [x] Dependency 'my-server.example.com/owner/repo' uses HTTP (insecure). Pass '--allow-insecure' to apm install, or run 'apm config set allow-insecure true' to allow HTTP dependencies globally.

The apm.yml file will record it in the following format:

dependencies:
  apm:
    - git: http://my-server.example.com/owner/repo
      allow_insecure: true

HTTP dependencies require two explicit signals:

allow_insecure: true on the dependency entry in apm.yml
either apm install --allow-insecure or apm config set allow-insecure true

The global configuration only removes the need to pass --allow-insecure on the CLI. It does not remove the allow_insecure: true requirement in apm.yml.

apm config set allow-insecure true

Fixes #636

TODO

Support for --allow-insecure flag when adding dependencies
Support for --allow-insecure flag when installing dependencies
Support for allow_insecure: true field in apm.yml and lockfile
Support for allow-insecure global config

Type of change

Bug fix
New feature
Documentation
Maintenance / refactor

Testing

Tested locally
All existing tests pass
Added tests for new functionality (if applicable)

arika0093 · 2026-04-14T08:43:42Z

@microsoft-github-policy-service agree

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR introduces an explicit opt-in flow for allowing HTTP (insecure) APM dependencies, including persisting that decision in apm.yml and preserving HTTP metadata during lockfile replays.

Changes:

Add --allow-insecure flag + global allow-insecure config to gate installation of http:// dependencies.
Persist and replay HTTP dependency metadata (is_insecure, allow_insecure) via manifest and lockfile.
Update docs and add unit tests covering HTTP parsing, lockfile round-trip, CLI behaviors, and cloning behavior.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/unit/test_install_update.py	Adds lockfile replay/round-trip tests for insecure HTTP metadata.
tests/unit/test_install_command.py	Adds CLI tests for `--allow-insecure` and install-time security checks.
tests/unit/test_config_command.py	Adds config command + config function tests for `allow-insecure`.
tests/unit/test_canonicalization.py	Adds parsing/serialization tests for HTTP dependency handling.
tests/unit/test_auth_scoping.py	Ensures HTTP deps don’t fall back to SSH during clone attempts.
src/apm_cli/models/dependency/reference.py	Introduces `is_insecure/allow_insecure`, HTTP canonicalization, manifest serialization, and scheme-aware URLs.
src/apm_cli/drift.py	Preserves insecure scheme info during lockfile replay.
src/apm_cli/deps/lockfile.py	Persists `is_insecure/allow_insecure` in lockfile read/write and dependency refs.
src/apm_cli/deps/github_downloader.py	Adds HTTP-only clone path and URL building for insecure deps.
src/apm_cli/config.py	Adds `get_allow_insecure` / `set_allow_insecure`.
src/apm_cli/commands/install.py	Adds `--allow-insecure`, manifest writing behavior, and install-time HTTP enforcement.
src/apm_cli/commands/config.py	Generalizes config get/set to support `allow-insecure`.
src/apm_cli/commands/_helpers.py	Propagates HTTP/artifactory/local fields when computing install paths.
src/apm_cli/bundle/plugin_exporter.py	Propagates HTTP/artifactory/local fields for exported install paths.
packages/apm-guide/.apm/skills/apm-usage/commands.md	Documents `--allow-insecure` in the command reference table.
docs/src/content/docs/reference/cli-commands.md	Documents `--allow-insecure` and new config key.
docs/src/content/docs/guides/dependencies.md	Documents HTTP dependency manifest format + caution guidance.

src/apm_cli/models/dependency/reference.py

src/apm_cli/commands/config.py

src/apm_cli/deps/github_downloader.py

- add install/config handling for HTTP dependencies behind explicit opt-in - preserve apm.yml manifest consistency by using git: entries - update tests and documentation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Also honor global allow-insecure config when adding HTTP dependencies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

sergio-sisternes-epam

Solid feature -- good security model with dual opt-in (dep-level + CLI/config), thorough test coverage (30+ tests), clean lockfile round-trip, and well-written docs. Nice work!

One blocking issue: to_apm_yml_entry() hardcodes allow_insecure = True, which could silently escalate security posture. See inline comment.

Also: this PR silently fixes a bug where artifactory_prefix was missing from DependencyReference construction in _helpers.py, plugin_exporter.py, and lockfile.py -- directly related to #614. Consider referencing that issue in the PR description.

sergio-sisternes-epam · 2026-04-14T10:39:52Z

src/apm_cli/models/dependency/reference.py

+                entry["ref"] = self.reference
+            if self.alias:
+                entry["alias"] = self.alias
+            entry["allow_insecure"] = True


Blocking: This hardcodes allow_insecure = True for all HTTP deps, ignoring self.allow_insecure. If this method is reused in a code path that hasn't applied the opt-in check, it silently grants insecure access.

Suggested change

entry["allow_insecure"] = True

entry["allow_insecure"] = self.allow_insecure

addressed in commit cd8bed0.

sergio-sisternes-epam · 2026-04-14T10:39:52Z

src/apm_cli/models/dependency/reference.py

            ado_project=ado_project,
            ado_repo=ado_repo,
            artifactory_prefix=artifactory_prefix,
+            is_insecure=dependency_str.startswith("http://"),


Suggestion: startswith("http://") is case-sensitive, but URL schemes are case-insensitive per RFC 3986. HTTP://host/repo would bypass insecure detection.

Suggested change

is_insecure=dependency_str.startswith("http://"),

is_insecure=urllib.parse.urlparse(dependency_str).scheme.lower() == "http",

addressed in commit 3c0122c.

sergio-sisternes-epam · 2026-04-14T10:39:53Z

src/apm_cli/deps/github_downloader.py

                return build_ssh_url(host, repo_ref)
            elif is_github and github_token:
-                # Only send GitHub tokens to GitHub hosts
+                # # Only send GitHub tokens to GitHub hosts


Nit: Double # — typo from the refactor commit.

Suggested change

# # Only send GitHub tokens to GitHub hosts

# Only send GitHub tokens to GitHub hosts

addressed in fe7179f.

sergio-sisternes-epam · 2026-04-14T10:39:53Z

src/apm_cli/models/dependency/reference.py

        sub_path = entry.get("path")
        ref_override = entry.get("ref")
        alias_override = entry.get("alias")
+        allow_insecure = bool(entry.get("allow_insecure", False))


Suggestion: bool(entry.get("allow_insecure", False)) treats any non-empty string (e.g., "false") as True. Since this is a security-relevant field, consider validating the type explicitly:

Suggested change

allow_insecure = bool(entry.get("allow_insecure", False))

allow_insecure = entry.get("allow_insecure", False)

if not isinstance(allow_insecure, bool):

raise ValueError("'allow_insecure' field must be a boolean")

addressed in commit 47b87ab.

…ence

…ences

…yReference

…ing for allow-insecure config

arika0093 · 2026-04-14T14:57:50Z

@sergio-sisternes-epam
Thanks for the review. I addressed the points you raised, so could you please take another look?

On #614: this PR now explicitly passes artifactory_prefix=dep.registry_prefix when reconstructing DependencyReference, which fixes a related missing-prefix propagation path. But I do not think it resolves the core #614 issue around _parse_artifactory_base_url() ignoring PROXY_REGISTRY_URL; that still appears to be unresolved.

arika0093 marked this pull request as ready for review April 14, 2026 08:43

arika0093 requested review from danielmeppiel and sergio-sisternes-epam as code owners April 14, 2026 08:43

Copilot AI review requested due to automatic review settings April 14, 2026 08:43

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Copilot started reviewing on behalf of arika0093 April 14, 2026 09:00 View session

arika0093 and others added 3 commits April 14, 2026 11:23

feat: support allow-insecure HTTP dependencies

1e0b2c2

- add install/config handling for HTTP dependencies behind explicit opt-in - preserve apm.yml manifest consistency by using git: entries - update tests and documentation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

fix: avoid SSH fallback for insecure HTTP deps

7d4718d

Also honor global allow-insecure config when adding HTTP dependencies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

refactor: simplify SSH and HTTPS handling for GitHub URLs

3a28c8c

sergio-sisternes-epam force-pushed the feat/allow-http-request branch from 101a07a to 3a28c8c Compare April 14, 2026 10:23

sergio-sisternes-epam requested changes Apr 14, 2026

View reviewed changes

arika0093 added 7 commits April 14, 2026 23:34

fix: trim whitespace in boolean value parsing

5260fbd

fix: remove redundant comment character in GitHub token handling

fe7179f

fix: enforce boolean type for allow_insecure field in DependencyRefer…

47b87ab

…ence

fix: normalize URL checks to be case-insensitive for dependency refer…

61b8340

…ences

fix: improve HTTP scheme detection for is_insecure field in Dependenc…

3c0122c

…yReference

fix: use allow_insecure field value in DependencyReference serialization

cd8bed0

test: add case-insensitive HTTP scheme detection and whitespace handl…

f6c2a38

…ing for allow-insecure config

	entry["allow_insecure"] = True
	entry["allow_insecure"] = self.allow_insecure

	is_insecure=dependency_str.startswith("http://"),
	is_insecure=urllib.parse.urlparse(dependency_str).scheme.lower() == "http",

	# # Only send GitHub tokens to GitHub hosts
	# Only send GitHub tokens to GitHub hosts

-        allow_insecure = bool(entry.get("allow_insecure", False))
+        allow_insecure = entry.get("allow_insecure", False)
+        if not isinstance(allow_insecure, bool):
+            raise ValueError("'allow_insecure' field must be a boolean")

Conversation

arika0093 commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODO

Type of change

Testing

Uh oh!

arika0093 commented Apr 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sergio-sisternes-epam left a comment

Choose a reason for hiding this comment

Uh oh!

sergio-sisternes-epam Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

arika0093 Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergio-sisternes-epam Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

arika0093 Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

sergio-sisternes-epam Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

arika0093 Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

sergio-sisternes-epam Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

arika0093 Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

arika0093 commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arika0093 commented Apr 14, 2026 •

edited

Loading

arika0093 Apr 14, 2026 •

edited

Loading