Skip to content

docs: clarify allow_insecure hardening guidance - #2636

Open
Ching Wei Kang (WilliamK112) wants to merge 2 commits into
microsoft:mainfrom
WilliamK112:docs/allow-insecure-hardening
Open

docs: clarify allow_insecure hardening guidance#2636
Ching Wei Kang (WilliamK112) wants to merge 2 commits into
microsoft:mainfrom
WilliamK112:docs/allow-insecure-hardening

Conversation

@WilliamK112

Copy link
Copy Markdown
Contributor

Description

Clarify the security guide's allow_insecure hardening advice so it matches
the implemented policy boundary.

This change:

  • states that apm-policy.yml has no dedicated allow_insecure key;
  • explains that dependency allow/deny rules match scheme-free canonical
    repository refs;
  • adds a restrictive dependencies.allow example and documents its limits;
  • distinguishes registry_source.allow_non_registry from HTTP Git transport;
  • points the hardening checklist to the existing two-gate runtime controls.

Fixes #2346

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (not applicable; documentation only)

Validation performed:

  • npm run test:links (14 tests passed)
  • npm run build (123 pages built; 969 relative links checked)
  • git diff --check

Spec conformance (OpenAPM v0.1)

  • N/A -- this PR does not change OpenAPM-observable behaviour.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the Enterprise Security guide to clarify how allow_insecure hardening works in practice, and to align the “Recommended hardening” checklist with the actual enforcement boundary (runtime gates vs. policy engine).

Changes:

  • Adds an explanation of the policy boundary around allow_insecure and how dependency allow/deny lists are intended to apply.
  • Adds a restrictive dependencies.allow example in the HTTP (insecure) dependencies section.
  • Rewords the hardening checklist item to point readers to the runtime gates section.
Suppressed comments (1)

docs/src/content/docs/enterprise/security.md:137

  • The canonicalization example appears inverted for policy matching: policy allow/deny lists are evaluated against the host-blind canonical dependency string, so http://mirror.example.com/contoso/tools would canonicalize to contoso/tools and would match contoso/**. Policy therefore cannot distinguish mirror hostname or HTTP vs HTTPS; only the runtime allow_insecure + --allow-insecure gates are scheme-aware.
For example, `http://mirror.example.com/contoso/tools` canonicalizes to
`mirror.example.com/contoso/tools`, so it does not match this allow list. If a
mirror's canonical host path is allowed, policy accepts that identity for both
HTTP and HTTPS. The manifest and CLI approvals above remain the scheme-aware
HTTP controls. `registry_source.allow_non_registry` is a separate,

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/src/content/docs/enterprise/security.md
@WilliamK112

Copy link
Copy Markdown
Contributor Author

Thanks for flagging the canonicalization example. I rechecked it against the current source and existing regression test. DependencyReference.to_canonical() strips only the configured default host (github.com); it preserves non-default hosts. The existing TestHttpInsecureDeps::test_http_to_canonical_is_scheme_free asserts that http://my-server.example.com/owner/repo becomes my-server.example.com/owner/repo, and that exact test passes on this head. Therefore http://mirror.example.com/contoso/tools does not match contoso/**. The policy still cannot distinguish HTTP from HTTPS for the same canonical host/path, which is why the following sentence keeps the manifest plus CLI gates as the scheme-aware controls. No documentation change is needed for this suppressed suggestion.

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.

[Documentation] allow_insecure discrepancy in Recommended hardening

2 participants