Skip to content

Add APM package-manager detection and command support - #51

Open
abhinavgautam01 wants to merge 2 commits into
git-pkgs:mainfrom
abhinavgautam01:apm-manager
Open

abhinavgautam01 wants to merge 2 commits into
git-pkgs:mainfrom
abhinavgautam01:apm-manager

Conversation

@abhinavgautam01

@abhinavgautam01 abhinavgautam01 commented Nov 12, 2026 •

Copy link
Copy Markdown

Closes #50.

Adds a definition for Microsoft APM, checked against the CLI help captured from APM 0.35.1 in references/apm/.

Detection

  • Manifest apm.yml, lockfiles apm.lock.yaml and legacy apm.lock. The current lockfile is listed first so it wins when conflicts are allowed.
  • New apm ecosystem, ordered after conda.
  • GitPkgsFileDetectionPolicy has a matching entry, so a directory with pyproject.toml and apm.yml returns both uv and apm from DetectAllFS.

Commands

Operation Command
init apm init --yes
install apm install, --frozen for frozen installs
add apm install owner/repo[#ref], --dev for development dependencies
remove apm uninstall owner/repo
list apm deps list
outdated apm outdated
update apm update [package] --yes
resolve apm deps tree

Notes:

  • apm list lists scripts, so dependency listing uses apm deps list.
  • init writes apm.yml in place, checked by running it in an empty directory.
  • update needs --yes because APM aborts the confirmation prompt without a TTY.
  • outdated was not in the issue but APM has it. It exits 0 whether or not anything is outdated.
  • No dependency command has JSON output, so json_output is not advertised. resolve returns the raw deps tree text. Parsing the lockfile is handled by the apm directory parser in resolve (Read APM dependency graphs from the lockfile resolve#17).
  • The minimum version is 0.35.1, the version tested.

Tests

apm_test.go loads the embedded definition and covers:

  • command translation for every operation, including frozen installs, development additions and adds with and without a ref
  • the advertised capabilities, with no json_output, path or vendor
  • detection from the manifest alone and from each lockfile name, with and without GitPkgsFileDetectionPolicy
  • DetectAllFS returning both uv and apm for pyproject.toml plus apm.yml

The README lists apm in the manager table and the init and resolve lists.

Summary by CodeRabbit

  • New Features
    • Added support for the APM package manager, including project detection from APM manifests and lockfiles, and commands for initializing, installing, managing, and resolving dependencies.
  • Bug Fixes
    • Package blocklist rules now also match package names with ref suffixes, such as owner/repo#main.
  • Documentation
    • Added APM command-line help for dependency management, installation options, updates, and other common workflows.

@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

This pull request adds APM package-manager detection and command support, including command mappings, capability declarations, tests, CLI reference text, and README entries. It also updates package blocklist matching to recognize names with trailing refs.

Changes: APM manager support

Layer / File(s) Summary
APM identity and detection
definitions/apm.yaml, detection_policy.go, apm_test.go, README.md
Adds APM version and detection metadata, registers apm.yml, apm.lock.yaml, and apm.lock, and tests detection with and without Python project files. The README lists APM as supported and documents its init and resolve support.
Command mappings and capabilities
definitions/apm.yaml, validate.go, apm_test.go
Maps supported operations to APM commands and declares capabilities. Adds package-name validation and tests for command translation, required arguments, accepted and rejected package names, and capabilities.
APM CLI reference text
references/apm/*
Adds APM command help text and records the CLI version as 0.35.1.

Ref-aware package blocklisting

Layer / File(s) Summary
Match ref-suffixed package names
policy.go, policy_test.go
Blocklist matching now checks the package name without a trailing #ref. Tests cover blocked refs and an unblocked package with a ref.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Possibly related PRs

  • git-pkgs/managers#48: Adds detection-policy and project-detection APIs used by the APM detection changes.

Suggested reviewers: andrew

Merge Risk: Minimal · up to 68500

This adds APM manager support and makes the package blocklist match names with a trailing #ref. No actionable merge-blocking risk is evident.

Security architecture risk: Low · up to 68500

APM reuses direct-process execution and rejects option-like package names. However, the shared blocklist change can turn a previous denial into an allowance for certain composite package selectors. Exposure depends on consumer-defined blocklist keys; no dependency-substitution exploit was demonstrated.

Retained concerns

  • Low · security · observed: The shared blocklist drops an existing normalization candidate when an @ suffix follows a # suffix. With only a#b configured as blocked, input a#b@c was denied at the base but is allowed at the head. This is a conditional enforcement-compatibility regression across consumers of the shared policy; real package resolution and deployed use of such keys remain unverified.
Security review details

Security Blast Radius

  • inferred — APM runs with the calling process's execution authority; the executor introduces no separate credential or sandbox boundary. The recorded CLI contract includes writes to manifests, lockfiles and agent-runtime configuration. The blocklist normalization change is shared across managers, but its effective exposure is limited to callers that configure the affected policy keys.

Security Findings and Attack Paths

  • observed — A caller-controlled non-flag argument can reach the changed matcher through PolicyRunner.Run. For a policy containing only the blocked key a#b, input a#b@c loses its previous matching candidate and passes this policy at the head. This establishes changed authorization behavior, not a demonstrated malicious dependency resolving to the same blocked asset.

Trust Boundaries and Controls

  • observed — The standard executor passes an argv array directly to exec.CommandContext rather than a shell. APM package validation occurs before argument insertion. Exact-string blocklist matches remain intact, and bare owner/repo keys now match the tested #main and #v1.2.0 forms. These controls limit the concern to the changed composite-selector normalization.
  • observed — The APM definition adds --yes defaults for init and update, but does not declare force, global installation, insecure transport or policy-bypass flags. The translator's raw Extra argument escape hatch remains available to direct callers and predates this PR; typed GenericManager operations inspected here do not populate it.

Resilience and Maintainability Implications

  • observed — Validation failures and enforcing-policy denials terminate before subprocess execution. Once execution begins, cancellation is delegated to exec.CommandContext and the result carries an exit code. Neither this path nor the APM definition establishes rollback of external file changes after failure or interruption.
Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "Add APM package-manager detection and command support" matches the main change. The PR adds an APM manager definition in definitions/apm.yaml, detection for apm.yml and both lockfile names …
Linked Issues check Passed Issue #50 is the only active directly linked issue. Issue #17 is closed and historical, so its criteria do not apply.

The previous review found #50 requirements met at the reviewed head and listed th…
Out of Scope Changes check Passed The core changes relate to the APM manager requested by #50: the definition, detection policy entry and constant, tests, README entries, and CLI help references. The outdated mapping is a related co…
Full details: Title check

Explanation

The title "Add APM package-manager detection and command support" matches the main change. The PR adds an APM manager definition in definitions/apm.yaml, detection for apm.yml and both lockfile names in detection_policy.go, command mappings, and tests in apm_test.go. The title does not mention the blocklist ref-suffix fix or the flag-rejecting validator, but the PR objectives describe those as secondary fixes, which the title does not need to cover.

Full details: Linked Issues check

Explanation

Issue #50 is the only active directly linked issue. Issue #17 is closed and historical, so its criteria do not apply.

The previous review found #50 requirements met at the reviewed head and listed them: detection for apm.yml, apm.lock.yaml, and apm.lock; command mappings for init, install (with --frozen), add (with optional #ref and --dev), remove, list (apm deps list), update (with --yes), and resolve (apm deps tree); no json_output capability; a GitPkgsFileDetectionPolicy entry; and tests for these cases including DetectAllFS with pyproject.toml plus apm.yml. The changes since the previous review (blocklist #ref matching, apm_package validator, and related tests) do not remove any of these requirements. Repository diffs for validate.go and the head-versus-previous range were unavailable, so the new validator and test changes are assessed from the PR summary only. The earlier conclusions for #50 are preserved.

The lockfile graph parser is deferred to git-pkgs/resolve#16/#17 as the issue permits. Resolve uses raw apm deps tree output, as #50 requests.

Full details: Out of Scope Changes check

Explanation

The core changes relate to the APM manager requested by #50: the definition, detection policy entry and constant, tests, README entries, and CLI help references. The outdated mapping is a related command of the same manager and is disclosed in the PR description.

The policy.go change adds trailing #ref stripping to PackageBlocklistPolicy.match. The diff was read directly. The code is generic and applies to all ecosystems. Its stated purpose is to match APM ref syntax such as owner/repo#main, and #50 requires optional refs for add. This is a related change. It alters shared policy behavior, so reviewers should confirm no other ecosystem uses # in package names that would now match a blocked bare name.

The apm_package validator in validate.go and its use for add, remove, and update relate to the flag-injection concern in #17 and to the APM argument forms used by this definition. The validator diff could not be read, so this assessment relies on the PR summary for it.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: aef24215-d17b-42e7-bffd-fd52e0b2bb83
Commits

Reviewing files that changed from the base of the PR and between 8a849f2 and 6850007.

Files selected for processing: 5

  • apm_test.go
  • definitions/apm.yaml
  • policy.go
  • policy_test.go
  • validate.go

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

@andrew andrew 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.

Thanks for this. The command mappings match the captured help and the APM docs, and the detection entries look right. Two things need changing before it can merge.

PackageBlocklistPolicy does not match owner/repo#ref. match in policy.go strips only a trailing @version, and APM is the first definition with a different version suffix. With owner/repo on the blocklist, an add with version main builds apm install owner/repo#main and is allowed in enforce mode. match needs to strip a trailing #ref as well, with a case for it in policy_test.go.

The package arg on add, remove and update in definitions/apm.yaml has no validate, so a package value of --force becomes apm install --force, which the help describes as deploying despite critical security findings. PolicyRunner skips dash-prefixed args, so no policy sees it either. The existing package_name validator is too narrow here because APM also accepts Git URLs, local paths and NAME@MARKETPLACE. Please add an apm_package validator in validate.go that rejects a leading -, reference it from those three args, and add a test for it.

@abhinavgautam01

Copy link
Copy Markdown
Author

Thanks, both fixed.

  • match now strips a trailing #ref before the @version check, so owner/repo#main hits an owner/repo entry. Cases added to TestPackageBlocklistScopedAndVersioned.
  • Added an apm_package validator that rejects a leading - and used it on the package arg of add, remove and update. TestValidateAPMPackage covers the validator. TestAPMRejectsFlagPackages checks that --force is refused while URLs, local paths and NAME@MARKETPLACE still build.

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.

Add APM package-manager detection and command support

2 participants