apm-action: 'apm install' before audit overwrites in-PR file tamper (drift detection blind)

[danielmeppiel]

Summary

microsoft/apm-action's default behavior runs apm install before any audit invocation. When a PR tampers with a deployed managed file (e.g. edits .github/instructions/secure-coding-base.instructions.md to weaken a security rule), apm install re-deploys the upstream version BEFORE the audit runs, silently overwriting the tamper. Audit then sees the pristine file and passes — even though the PR ships malicious content.

Repro

1. Repo wires microsoft/apm-action@v1 with default settings.
2. PR edits one line of a deployed instruction file under .github/instructions/ or .github/hooks/.
3. Action runs: apm install overwrites the tampered file from the upstream plugin source.
4. apm audit runs against the now-pristine file. PASSES.
5. PR merges. Tampered content is in main's history but not in the deployed tree — until someone runs apm install locally and the PR's edits are silently lost.

Either outcome (merge of bad intent into history; or silent loss of the edit) is broken.

Suggested fix

Two options, both better than today:

A. Add audit-only mode (recommended). Expose an action input that runs the apm CLI install (so audit can use it) but does NOT re-deploy files. Audit sees the as-committed file state, drift / content-integrity catch the tamper.

B. Default behavior change. apm install in CI should be a no-op when apm.lock.yaml is present and the deployed file hashes match what's in the lockfile. Tamper is a hash mismatch; re-deploying silently overwriting it is the bug.

Workaround (proven)

Wire your own reusable workflow that uses microsoft/apm-action with setup-only: true (apm CLI on PATH, no install/deploy), then runs apm audit --ci --no-drift directly. content-integrity check then sees the tamper via lockfile hash drift and fails the gate. Working example: DevExpGbb/zava-agent-config/.github/workflows/apm-audit.yml@v5.1.2.

Why this matters

Drift detection is the WHOLE POINT of deployed_file_hashes in the lockfile (https://microsoft.github.io/apm/guides/drift-detection/). The default action behavior makes that detection structurally impossible in CI. Most consumers will use the action by default and assume it audits what's committed. It does not.

Context

Discovered while building a 5-beat governance demo for an enterprise pilot. Beat 5 specifically demos a malicious instruction-file edit; surfacing it required moving off the action's default invocation.

[github-actions]

Triage decision

accept

Proposed labels

theme/security
area/audit-policy
area/ci-cd
type/bug
status/accepted
priority/high

Milestone

0.12.5 (recommended; unable to apply programmatically -- milestone number not resolvable without authenticated CLI)

Note: area/docs-site is also implied (apm-action docs and drift detection guide need updating for the new audit-only mode) but was pruned to stay within the 6-label cap. Please add manually if desired.

Suggested next action

Add an audit-only input to microsoft/apm-action that installs the CLI and populates the install cache but does NOT re-deploy files, so apm audit --ci runs against the as-committed file state.

Suggested issue comment

Thanks for the detailed repro sequence and the working workaround reference -- this is a confirmed structural blind spot in the default `apm-action` behavior.

The root cause is ordering: `apm install` re-deploys upstream files from plugin sources before `apm audit` runs. Any in-PR tamper to a deployed managed file is overwritten by install, so audit sees the pristine version. The `content-integrity` check (lockfile hash comparison) can catch this IF it runs against the as-committed file state -- but the default action behavior makes that impossible.

**Accepted.** Option A (`audit-only` mode) is the right fix: expose an action input that runs `apm install` to populate the cache and put the CLI on PATH, but does not re-deploy files. `apm audit --ci` then sees the as-committed file state and `content-integrity` / drift detection can do their jobs.

Option B (behavior change: no-op if lockfile hashes match) is also valid as a defense-in-depth change but is a riskier default to flip; it should be considered as a follow-on, not a replacement for Option A.

The existing `setup-only: true` + `--no-drift` workaround (referenced in the report) confirms the approach works. `audit-only` mode would make this the first-class, discoverable path.

Documentation for `apm-action` and the drift detection guide ((microsoft.github.io/redacted) will need updates to describe the new mode and recommend it for PR tamper-detection workflows.

Target: 0.12.5 patch alongside #1199 and #1198 for the governance defense-in-depth story.

Per-lens notes (collapsed)

DevX UX Expert -- User-Need Reviewer

The user expectation for microsoft/apm-action is "wire this in my CI and get governed." The default action behavior silently defeats the primary value proposition: PR tamper detection. Any org that wired apm-action with default settings and assumed it audits what's committed in the PR is currently unprotected. The audit-only mode makes the right behavior discoverable without requiring users to read the drift detection guide and construct a custom workflow. The workaround (custom reusable workflow with setup-only: true + --no-drift) is valid but not discoverable.

Supply Chain Security Expert -- Risk-Surface Reviewer

This is the most impactful security issue in this sweep. The default apm-action behavior makes deployed_file_hashes (lockfile content-integrity) and drift detection structurally blind in PR CI. An attacker who knows this can edit any deployed instruction file in a PR, pass the audit gate, and merge malicious governance content. theme/security, area/audit-policy, area/ci-cd are all required. priority/high is warranted -- this is not a theoretical risk; it requires only a PR edit to exploit.

OSS Growth Hacker -- Contributor-Tone Reviewer

Not activated -- danielmeppiel is a COLLABORATOR with extensive prior interactions.

Python Architect -- Architecture Reviewer

Not activated -- the fix is a new action input (audit-only) that conditionally skips the file-deploy step of apm install. No cross-cutting Python architecture decision; the install flow already has a --setup-only mode (per the workaround reference). Exposing it as a first-class action input is an interface addition, not a design change.

Doc Writer -- Documentation Reviewer

Activated. Two documentation surfaces need updates: (1) the microsoft/apm-action README (new audit-only input, recommended usage for PR tamper-detection workflows); (2) the drift detection guide at (microsoft.github.io/redacted) (explain why audit-only mode is required for drift to see in-PR tampers). area/docs-site is implied but pruned from labels to stay within the 6-label cap -- maintainers should add it manually if they want the label. The suggested comment wording is grounded in the user vocabulary from the drift detection guide.

APM CEO -- Triage Arbiter

This is the highest-impact issue in this sweep: the default action behavior defeats the central governance value proposition for every org using apm-action with default settings. priority/high is non-negotiable. Option A (audit-only mode) is the correct first-class fix; Option B (behavior change) can follow. The fix should land with the #1198/#1199 cluster to tell a coherent "governance defense-in-depth" story in the 0.12.5 release notes. Note that area/docs-site was pruned from the label set to respect the 6-label cap; maintainers may add it.

{
  "decision": "accept",
  "decision_detail": "",
  "theme": "theme/security",
  "areas": ["area/audit-policy", "area/ci-cd"],
  "type": "type/bug",
  "status": "status/accepted",
  "priority": "priority/high",
  "preserved_labels": [],
  "milestone": "0.12.5",
  "next_action": "Add audit-only mode to microsoft/apm-action that installs CLI and cache without re-deploying files, making PR tamper detection structurally possible with default action usage.",
  "comment_markdown": "Thanks for the detailed repro sequence and the working workaround reference -- this is a confirmed structural blind spot in the default `apm-action` behavior.\n\nThe root cause is ordering: `apm install` re-deploys upstream files from plugin sources before `apm audit` runs. Any in-PR tamper to a deployed managed file is overwritten by install, so audit sees the pristine version. The `content-integrity` check (lockfile hash comparison) can catch this IF it runs against the as-committed file state -- but the default action behavior makes that impossible.\n\n**Accepted.** Option A (`audit-only` mode) is the right fix: expose an action input that runs `apm install` to populate the cache and put the CLI on PATH, but does not re-deploy files. `apm audit --ci` then sees the as-committed file state and `content-integrity` / drift detection can do their jobs.\n\nOption B (behavior change: no-op if lockfile hashes match) is also valid as a defense-in-depth change but is a riskier default to flip; it should be considered as a follow-on, not a replacement for Option A.\n\nThe existing `setup-only: true` + `--no-drift` workaround (referenced in the report) confirms the approach works. `audit-only` mode would make this the first-class, discoverable path.\n\nDocumentation for `apm-action` and the drift detection guide ((microsoft.github.io/redacted) will need updates to describe the new mode and recommend it for PR tamper-detection workflows.\n\nTarget: 0.12.5 patch alongside #1199 and #1198 for the governance defense-in-depth story."
}

---

Triage status: agentic proposal pending human ratification.
Silence is approval. Maintainers can:

• Override any label or milestone above by editing it directly --
  human edits are authoritative and will not be reverted on
  subsequent runs.
• Re-trigger triage by applying the status/needs-triage label, or
  by removing status/triaged to enroll the issue in the next
  daily sweep.

Posted by the Triage Panel workflow. See .apm/skills/apm-triage-panel for the panel skill.

Note

🔒 Integrity filter blocked 12 items

The following items were blocked because they don't meet the GitHub integrity level.

• #1120 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1122 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1130 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1138 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1156 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1157 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1195 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1203 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1209 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1210 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1220 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
• #1222 search_issues: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Triage Panel · ● 1.1M · ◷