Repository navigation
fix(engine): allow every subcommand of Copilot CLI stem commands - #2325
Merged
Merged
Conversation
Copilot CLI parses some commands (git, npm, pip, dotnet, cargo, curl, ...) as stem commands. For those, `--allow-tool "shell(npm)"` permits only a bare `npm` with no arguments, so every real invocation was refused with denied-no-approval-rule. That silently broke the npm/npx, pip, and dotnet commands the node, python, and dotnet runtimes add to the allow-list, the git commands the PR execution-context contributor adds, and any author entry such as `bash: [git]` or `bash: [curl]`. Emit `shell(<cmd>:*)` for bare stem commands, mirroring gh-aw's CopilotStemCommands list. Entries with `:` or a space pass through unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
jamesadevine
added a commit
that referenced
this pull request
Oct 11, 2026
Both rested on wrong diagnoses of why Copilot CLI refused npm in the live package-feed smoke. The actual cause is that ado-aw emitted shell(npm) for a Copilot CLI stem command, which permits only a bare `npm` (fixed in #2325, cherry-picked next). Restore the symlinked wrappers and remove the edit:false warning and its docs note. The smoke prompt keeps the instruction not to chain extra commands onto the commands under test. Reverts add0c43 and a8db2a5. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550
This was referenced Oct 11, 2026
jamesadevine
added a commit
that referenced
this pull request
Oct 16, 2026
Both rested on wrong diagnoses of why Copilot CLI refused npm in the live package-feed smoke. The actual cause is that ado-aw emitted shell(npm) for a Copilot CLI stem command, which permits only a bare `npm` (fixed in #2325, cherry-picked next). Restore the symlinked wrappers and remove the edit:false warning and its docs note. The smoke prompt keeps the instruction not to chain extra commands onto the commands under test. Reverts add0c43 and a8db2a5. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550
|
@jamesadevine we hit this in our pipelines, can we release a v0.54.0 to have this fix? |
jamesadevine
added a commit
that referenced
this pull request
Oct 27, 2026
…ns.packages (#2315) * feat(compile)!: credential-isolated package feed access via permissions.packages Agents can restore npm, PyPI, NuGet, and Cargo packages from Azure Artifacts feeds without holding a feed credential. The trusted ado-proxy container holds the build identity or a WIF-minted token and attaches it only to GET/HEAD requests for granted feeds, views, and protocols. - permissions.packages front matter (feeds, protocols, view, upstream, identity-role, optional service connection) and permissions-required.packages - ado-proxy catalog v2 package family with a separate credential slot, host-side feed GUID resolution and access preflight, blob-only redirect relay - per-process package-manager wrappers (npm, npx, pip, pip3, uv, dotnet, cargo) - runtimes.<x>.feed source selection and runtimes.<x>.public-registry: block - audit rollups by family/protocol and an upstream-unauthorized finding - codemod 0009 migrates Azure Artifacts runtimes.<x>.feed-url BREAKING CHANGE: Azure Artifacts runtimes.<x>.feed-url is replaced by permissions.packages plus runtimes.<x>.feed. Codemod 0009 migrates existing sources automatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * fix(ado-proxy): relay vsassets.io package downloads and preflight ReadPackages Live probes against Azure Artifacts showed two gaps: - npm, PyPI, and NuGet downloads redirect (303) to <shard>vsblobprodcus<n>.vsblob.vsassets.io, which the package redirect allowlist did not include, so every real download would have been refused with redirect-denied. Add the .vsblob.vsassets.io suffix (already covered by the core AWF *.vsassets.io allowlist entry). - The feed GET used by resolve-feeds answers 200 to an identity without the ReadPackages permission, so the preflight passed while every restore would fail. Preflight now lists one package, which requires ReadPackages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * test(smoke): add smokeCases filter to the candidate orchestrator A comma-separated smokeCases runtime parameter narrows a candidate run to the named cases. Blank (the default, and every scheduled or comment-triggered run) still runs every case. Unknown ids, or ids outside the mode, fail the run rather than silently running less. The value reaches the harness only as the SMOKE_CASES environment variable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * test(smoke): live package-feed smokes for build identity and WIF Register package-feeds (build identity) and package-feeds-wif (agent-playground-read) against the AgentPlaygroundTestFeed feed. Each restores through npm, pip, and dotnet via ado-proxy and proves no package credential reaches the sandbox. The build-identity case also proves ungranted feeds are refused and public-registry: block removes the public registry. The compiler test now checks every pipelineText assertion cases.json declares for these cases, so a compiler change that breaks one fails cargo test rather than the nightly smoke. REGISTERED.md records the feed, upstreams, and grants the cases depend on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * test: cover feed exclusivity and assert streamed-body truncation strictly Review follow-ups on #2315: - validate_feed_exclusivity had no direct test. Add unit tests for both conflicting combinations plus a per-runtime check through parse_markdown and each runtime extension. - The "cuts off a streamed body" e2e test swallowed rejections and only asserted when the request resolved. It now accepts exactly two outcomes, a partial body bounded by the limit or a connection reset, and requires a new response-too-large decision record either way. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * fix(compile): warn that edit: false blocks package managers; fix smoke prompts Live run 646826 showed Copilot CLI refusing every npm, pip, and dotnet invocation when tools.edit is false: without --allow-all-paths the CLI denies commands that write caches and temporary files outside the workspace, and the agent cannot request approval non-interactively. - permissions.packages now warns at compile time when tools.edit is false. - The package-feed smokes keep edit enabled (the default) and tell the agent not to chain extra commands onto the ones under test. - docs/package-feeds.md records the limitation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * fix(compile): install package-manager wrappers as regular files Live run 646837 still saw Copilot CLI refuse every wrapped npm command with --allow-tool "shell(npm)" and --allow-all-paths present, while unwrapped tools ran. The wrappers were symlinks to .ado-aw-package-wrapper; the working az wrapper is a regular file. Install one regular-file copy per tool name so the executable Copilot CLI resolves carries the allowed name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * revert: drop the edit:false warning and the wrapper copy change Both rested on wrong diagnoses of why Copilot CLI refused npm in the live package-feed smoke. The actual cause is that ado-aw emitted shell(npm) for a Copilot CLI stem command, which permits only a bare `npm` (fixed in #2325, cherry-picked next). Restore the symlinked wrappers and remove the edit:false warning and its docs note. The smoke prompt keeps the instruction not to chain extra commands onto the commands under test. Reverts add0c43 and a8db2a5. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * test(smoke): let npm pack write to the working directory; explain variable substitution Run 647310 proved the build-identity npm path end to end: three package GETs through ado-proxy, all allowed with 2xx upstream responses, including the 3.7 KB is-number tarball. npm then failed only because --pack-destination must already exist and the agent has no mkdir. Write the tarball to the working directory instead. The WIF agent stopped before running any check: it compared its prompt with the repository copy, saw $(Build.BuildId) replaced by a literal ID, and took that for prompt injection. Both prompts now say up front that pipeline variables are substituted and that repository files are not to be compared. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * fix(compile): make pip trust the package proxy through SSL_CERT_FILE Run 647315 proved npm end to end with both credentials (build identity and WIF), but pip failed with SSLCertVerificationError against the proxy's leaf. Recent pip builds its TLS context with truststore (the system trust store) and PIP_CERT did not take effect; on Linux truststore reads OpenSSL's default verify paths, which honour SSL_CERT_FILE. The pip wrapper now sets SSL_CERT_FILE to the same system-roots-plus-proxy-CA bundle, still scoped to the one process. PIP_CERT stays for pip's legacy-certs mode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * fix(compile): route NuGet through the package proxy; anchor smoke prompts Run 647325 proved npm and PyPI end to end with the WIF credential, including the npm tarball's 3xx relay to *.vsblob.vsassets.io. NuGet restore then failed with NU1301: Squid logged NuGet's CONNECT to pkgs.dev.azure.com coming straight from the agent, not from ado-proxy. NuGet takes its proxy from http_proxy even for HTTPS feeds, and that variable still named Squid. The dotnet wrapper now points http_proxy and HTTP_PROXY at the engine too. The engine forwards other cleartext to Squid and refuses cleartext to intercepted hosts. The build-identity agent ran no checks: it found its prompt as a repository file and decided the message was not its task. Both smoke prompts now open by saying this is the task, that the repo copy is expected, and that the repository is not to be inspected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * fix(compile): keep compiler-owned wrapper dirs first on the agent PATH Run 647387 still saw NuGet reach pkgs.dev.azure.com straight from the agent: dotnet never ran the package wrapper. AWF's entrypoint builds the agent PATH from the host PATH (which carries our $GITHUB_PATH prepends) and then prepends $DOTNET_ROOT, $CARGO_HOME/bin, $JAVA_HOME/bin, and others, so the real dotnet shadowed the wrapper; cargo would have done the same. Re-assert every awf_path_prepends directory at the front of PATH inside the engine command, which runs after the entrypoint. Entries are compiler-owned and character-checked because they are spliced into a single-quoted shell word. The package-feed smokes also pass --no-custom-instructions: the ado-aw repository's AGENTS.md otherwise loads as Copilot custom instructions and cast the agent as a repository contributor, which made it decline the smoke in several runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * fix(compile): public-registry block removes every entry that admits the registry Run 647440 proved the whole package path with both credentials: npm, pip, and NuGet restores through ado-proxy, the ungranted feed refused with 403, and no credential in the sandbox (the WIF case passed every check). The one failure: `public-registry: block` left registry.npmjs.org reachable, because the node ecosystem also allows the bare apex npmjs.org and AWF matches subdomains. Removal now drops any allowlist entry that would admit a blocked host under AWF's subdomain matching, and the npm block list names the npmjs.org, npmjs.com, and yarnpkg.com apexes explicitly. A new test compiles all three runtimes blocked (plus covering network.allowed entries) and asserts no remaining entry admits any blocked host. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * docs(package-feeds): record live smoke coverage Full candidate run 647482 passed every case, including package-feeds (build identity) and package-feeds-wif, so the "needs live validation" limitation is replaced by a description of what the smoke suite proves. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * fix: address /review follow-ups on package feeds - resolve-feeds: a stdin read failure now returns the structured ##vso[task.logissue] error (exit 1) instead of rejecting with a raw stack trace; covered by a test. - bash lint: "Install package-manager wrappers (ado-proxy)" joins REQUIRED_STEP_DISPLAY_NAMES, so losing that fixture coverage fails the test. - codemods: add a warnings channel. Codemods stay pure: CodemodContext::warn records a note, CodemodReport::warnings carries it, and compile prints it under the "applied codemods" list. - codemod 0009 uses it when it migrates a feed-url onto an author's existing view-less grant that does not set `upstream: allow`, since the agent can no longer pull new upstream packages through it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * feat(compile): warn on legacy Azure Artifacts URLs in committed package config The package proxy only intercepts pkgs.dev.azure.com, so a committed nuget.config, .npmrc, pip.conf, uv.toml, pyproject.toml, .yarnrc.yml, or .cargo/config(.toml) that still names {org}.pkgs.visualstudio.com sends restores to Azure Artifacts with no credential: they fail with 401 and nothing says why. Codemod 0009 already rewrites front matter; this covers the files the compiler does not own. A new warning-only check scans those files at the source repository's git root, finds legacy-host URLs (including npm's protocol-relative //host keys), and suggests the canonical pkgs.dev.azure.com/{org}/... URL. It runs only when the workflow can restore packages (permissions.packages or a node/python/dotnet runtime). compile prints it, and `ado-aw lint` and the lint_workflow MCP tool report it as `legacy-feed-url` warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * feat(compile): warn on committed feed URLs the package proxy would refuse Extends the committed package-config check: besides legacy-host URLs, it now reports pkgs.dev.azure.com feed URLs in nuget.config, .npmrc, pip.conf, and the rest that permissions.packages does not cover. The match mirrors the proxy's authorizePackageRequest. Each finding names its reason: feed-not-granted (organization, project, feed; an organization-scoped grant never covers a project-scoped URL), view-not-granted, protocol-not-granted, unknown-route, or no permissions.packages at all. It never reports a URL the proxy would allow. GUID identifiers and grants without organization cannot be decided before runtime resolution, so they are treated as matching. compile prints the warnings, and `ado-aw lint` and the lint_workflow MCP tool report them as `ungranted-feed-url`. Also fixes the AGENTS.md entry from the previous commit, where escaping had turned the backticks around `legacy-feed-url` into backslashes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 * fix(compile): reject malformed package feed URL escapes Use shared strict percent-decoding so malformed package feed URL escapes are rejected consistently. Strengthen package-wrapper and service-connection regression assertions to cover the corrected behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2118822c-f81d-4b93-a8a9-662fca1d23fb --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b0ab0d4-23f4-4a89-90ea-3c40758ea550 Copilot-Session: 2118822c-f81d-4b93-a8a9-662fca1d23fb
Collaborator
Author
|
@JamesBurnside I release v0.54.0 yesterday - let me know how you get on! It has special configuration for package feeds now. Please give it a go and let me know of any issues! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Copilot CLI parses some commands as stem commands:
git,npm,pip,dotnet,cargo,curl, and others. For a stem command, the rule--allow-tool "shell(npm)"permits only a barenpmwith no arguments. Everyreal invocation, even
npm --version, is refused withdenied-no-approval-rule.ado-aw emitted
shell(<cmd>)for everytools.bashentry, which broke thesecases:
npm/npx,pip, anddotnetcommands that the node, python, anddotnet runtimes add to the allow-list;
gitcommands the PR execution-context contributor adds;bash: [git],[curl], or[cargo].Commands Copilot doesn't treat as stems (
az,node,python,uv,pip3,cat, …) were unaffected. That is why the existingazsmoke passes.This PR emits
shell(<cmd>:*)for bare stem commands, mirroring gh-aw'sCopilotStemCommandslist (pkg/constants/engine_constants.go) and its rulegeneration in
pkg/workflow/copilot_engine_tools.go. Entries containing:or a space (an explicit pattern or a named subcommand) pass through unchanged.
Evidence
Live (AgentPlayground, feat(compile)!: credential-isolated package feed access via permissions.packages #2315 package-feed smoke builds 646838 and
646851): the agent was refused on every
npmcall despite--allow-tool "shell(npm)". Its OTel span recordedgithub.copilot.permission.result = denied-no-approval-rule-and-could-not-request-from-user.In the same runs,
node -e …(shell(node)) ran.Local (Copilot CLI 1.0.93, non-interactive
-p):shell(npm)npm --versionshell(npm:*)npm --versionshell(npm *)npm --versionshell(node)node --versionFound while live-testing #2315. That PR needs this fix to run its smoke.
Test plan
cargo build: passedcargo test --no-fail-fast: passed (3720 passed, 0 failed, 2 ignored)stem_commands_allow_every_subcommand: rule mapping, includingpass-through cases
runtime_package_managers_get_stem_rules: node, python, and dotnetruntimes emit
shell(npm:*),shell(npx:*),shell(pip:*), andshell(dotnet:*), and never the bare formshell(git:*)cargo clippy --all-targets: passedENFORCE_BASH_LINT=1 cargo test --test bash_lint_tests: passedgit diff --check: cleanLive confirmation comes from the #2315 package-feed smoke rerun, which
cherry-picks this commit.