fix: hint to host-qualify cross-repo on *.ghe.com (closes #1305)#1319
Conversation
) PR microsoft#1292 fixed the silent ``github.com`` auth fallback for **in-marketplace** plugin sources on ``*.ghe.com`` marketplaces but deliberately scoped its host backfill via ``_is_in_marketplace_source`` to avoid changing routing for cross-repo dict sources. A bare cross-repo ``repo: owner/proj`` on an enterprise marketplace still legitimately means two different things -- a real ``github.com`` open-source dep, or a misconfigured same-host entry that should have been ``corp.ghe.com/owner/proj`` -- and the resolver cannot disambiguate them. The silent mis-route survives for the second intent: the canonical stays bare, ``DependencyReference.parse`` defaults the host to ``github.com``, and the install path reports the generic ``not accessible or doesn't exist -- run with --verbose for auth details`` with zero pointer at the marketplace's enterprise host. Surface the diagnostic at the install-time validation-failure boundary, not at resolver time. The legitimate cross-host case validates successfully and never sees a hint; the misconfigured case fails validation and gets an actionable host-qualified suggestion. The resolver-time always-on warning the PR microsoft#1292 review panel rejected -- which would false-positive on the legitimate case and train operators to ignore -- is avoided. Approach ======== Resolver attaches a typed ``CrossRepoMisconfigRisk`` sentinel to ``MarketplacePluginResolution`` when **all** of: - ``dependency_reference`` is ``None`` (GitHub-family virtual-shorthand path; GitLab-class and self-managed FQDN marketplaces build a structured ref upstream and sidestep the bug) - ``plugin.source`` is a dict whose normalized type is ``github`` -- via the existing ``_coerce_dict_plugin_type`` (covers ``type``/``kind``/``source`` synonyms plus the inferred-github fallback). Cross-repo ``gitlab`` / ``git-subdir`` dict sources on enterprise marketplaces hit the same auth-routing bug but the "host-qualify with marketplace host" remediation only matches operator intent for the GitHub family. - the source is NOT an in-marketplace reference (PR microsoft#1292's domain) - ``_needs_canonical_host_prefix`` agrees the canonical is bare and the host is GitHub-family enterprise (``*.ghe.com``; idempotent against already host-qualified, URL, and SSH forms) - the ``repo`` field is a non-empty ``owner/repo`` shorthand The helper is pure -- no logging, no canonical mutation. Resolver behavior is unchanged; only the resolution object carries one extra optional field. Install command records the risk in a per-call ``_misconfig_risks`` dict **before** validation runs. The existing ``_marketplace_provenance`` map only gets written on validation success and cannot be relied on at the failure boundary. When ``_validate_package_exists`` returns ``False`` (which is how the GitHub-family auth failure surfaces -- ``AuthResolver.try_with_fallback`` collapses 401/404/network into a single ``False``, no typed ``AuthenticationError``), the validation-fail branch emits the hint inline via ``logger.info`` so the operator can correct ``marketplace.json`` without rerunning under ``--verbose`` to decode the auth trace. Why this layer, not ``AuthenticationError`` =========================================== The two ``raise AuthenticationError`` sites in the install pipeline are both gated to non-GitHub hosts (ADO / self-managed): ``pipeline.py`` preflight skips ``is_github_hostname(host)`` early; ``validation.py`` requires the ``is_ado_auth_failure_signal`` stderr pattern. The github.com fallback path goes through ``try_with_fallback`` which returns ``False`` on failure, and the caller records ``(canonical, reason)`` into ``invalid_outcomes``. Decorating ``AuthenticationError`` would be a dead-code hook for this bug -- the typed exception never fires on the github.com path. The validation-fail branch is the actual choke point. Scope and tradeoffs =================== - 404 typo on the cross-repo ``repo`` field and network failures will also trigger the hint; the wording leads with the routing fact ("resolved to 'github.com'") and the suggestion is conditional ("If you meant the enterprise host"), so the false-positive remains advisory rather than misleading. Distinguishing 401 from 404/network here would require threading HTTP status out of ``try_with_fallback`` -- a much broader cross-cutting change. - The silent-success-on-wrong-host case (cross-repo bare where the same ``owner/repo`` happens to exist on github.com with different content) cannot be detected without changing the routing semantics PR microsoft#1292 preserved. This is acknowledged out of scope in the issue. Tests ===== ``TestCrossRepoMisconfigRisk`` (resolver, 14 cases) locks the truth table for sentinel attach / no-attach across the dict-type synonyms (``type``, ``kind``, ``source``, inferred-github), host-qualified / URL / SSH / no-slash defensive guards, the gitlab / git-subdir exclusion, and pure ``github.com`` marketplace non-pollution. ``TestResolvePackageReferencesCrossRepoMisconfigHint`` (install, 4 cases) locks the hint emission contract: hint fires only when a risk-bearing marketplace resolution subsequently fails validation; the legitimate cross-host path that validates successfully emits no hint; in-marketplace and plain owner/repo failures emit no hint. Both test suites were toggle-verified -- removing the resolver helper call or the install-side emission block makes the corresponding positive case fail.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 1 | 1 | Solid sentinel + collect-then-render pattern; one defensive getattr undermines the type contract; hint log-level mismatch is a nit. |
| CLI Logging Expert | 0 | 2 | 1 | Use logger.warning() not logger.info() for the misconfiguration hint; strip the redundant 'Hint:' prefix; lead with the actionable fact per traffic-light convention. |
| DevX UX Expert | 0 | 1 | 2 | Hint message copy is clear and actionable; logger.info may be silenced at default verbosity, hiding the hint from the operator who needs it most. |
| Supply Chain Security Expert | 0 | 1 | 1 | Hint-only detection leaves the dependency-confusion vector (pre-staged github.com package) silent on the success path; enterprise hostname appears in log output. |
| OSS Growth Hacker | 0 | 2 | 1 | Solid enterprise friction-reduction; hint text is self-serve-ready but one word swap and a CHANGELOG beat would maximize enterprise onboarding ROI. |
| Auth Expert | 0 | 1 | 2 | Sentinel gates are correct; advisory: hint misfires on non-auth failures of legitimate github.com cross-host deps (rate limits, timeouts, expired PATs). |
| Doc Writer | 0 | 2 | 0 | manifest-schema.md documents source as bare owner/repo with no GHE host-qualification note; the new hint has no corresponding doc anchor. |
| Test Coverage Expert | 0 | 2 | 1 | 13+4 unit tests cover all condition branches; integration regression trap misses the new cross_repo_misconfig_risk field assertion. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
-
[CLI Logging + Python Architect + DevX UX (3-persona convergence)] (blocking-severity) Replace
logger.info()withlogger.warning()for the misconfiguration hint; remove the redundant 'Hint:' prefix; inline the resolved enterprise hostname in the message body. -- Three independent reviewers flagged the same defect: info-level output is invisible at default verbosity, making the PR's stated goal a no-op for most operators. One-liner fix; zero risk. -
[Test Coverage Expert] Add
assert result.cross_repo_misconfig_risk is not Noneto the existing integration regression traptest_cross_repo_locks_known_silent_misroute; add a new integration scenariotest_cross_repo_hint_emitted_on_validation_failure. -- Both carryevidence.outcome='missing'onsecure-by-defaultsurfaces; under the panel's evidence-weighting rules, missing-test rows on critical surfaces rank above recommended opinion findings. -
[Auth Expert] Amend the hint message to acknowledge the transient-failure alternative: add a clause such as 'If this is intentionally a github.com dependency, verify your github.com credentials and that the repository is accessible.' -- The sentinel attaches to both the misconfigured path and legit cross-host deps that fail for transient reasons (429, timeout, expired PAT); without this clause the hint actively misdirects operators in the legitimate-dep-transient-failure case.
-
[Supply Chain Security Expert] Open a follow-up issue: emit a lower-severity warning at resolution time whenever
cross_repo_misconfig_riskis non-None, even on the successful validation path, to close the dependency-confusion silent-pass gap. -- A pre-stagedgithub.compackage causes_validate_package_existsto return True on the wrong host; this is a real supply-chain vector that should be tracked as a hardening item. -
[Doc Writer] Add a GHE host-qualification callout to
manifest-schema.mdunder thesourcefield, and add a 'Marketplace host mismatch' subsection to the install-failures troubleshooting page. -- The hint tells operators to qualify therepofield but provides no doc anchor; without a corresponding doc update, enterprise operators who search for the error symptom will find nothing.
Architecture
classDiagram
direction LR
class MarketplacePluginResolution {
<<ValueObject>>
+canonical str
+plugin MarketplacePlugin
+dependency_reference DependencyReference | None
+cross_repo_misconfig_risk CrossRepoMisconfigRisk | None
}
class CrossRepoMisconfigRisk {
<<ValueObject>>
+marketplace_host str
+bare_repo_field str
+suggested_qualified_repo str
}
class MarketplacePlugin {
<<Entity>>
+source dict | str
}
class MarketplaceSource {
<<ValueObject>>
+host str
+owner str
+repo str
}
class DependencyReference {
<<ValueObject>>
+parse() DependencyReference
}
class _compute_cross_repo_misconfig_risk {
<<Pure>>
+__call__(plugin, source, canonical, dep_ref) CrossRepoMisconfigRisk | None
}
class resolve_marketplace_plugin {
<<IOBoundary>>
+__call__() MarketplacePluginResolution
}
class _resolve_package_references {
<<IOBoundary>>
+_misconfig_risks dict
}
class CrossRepoMisconfigRisk:::touched
class MarketplacePluginResolution:::touched
class _compute_cross_repo_misconfig_risk:::touched
class resolve_marketplace_plugin:::touched
class _resolve_package_references:::touched
MarketplacePluginResolution *-- CrossRepoMisconfigRisk : carries sentinel
MarketplacePluginResolution *-- MarketplacePlugin : wraps
MarketplacePluginResolution o-- DependencyReference : may carry
_compute_cross_repo_misconfig_risk ..> MarketplacePlugin : reads source
_compute_cross_repo_misconfig_risk ..> MarketplaceSource : reads host
_compute_cross_repo_misconfig_risk ..> CrossRepoMisconfigRisk : returns
resolve_marketplace_plugin ..> _compute_cross_repo_misconfig_risk : delegates
resolve_marketplace_plugin ..> MarketplacePluginResolution : returns
_resolve_package_references ..> resolve_marketplace_plugin : calls
_resolve_package_references ..> CrossRepoMisconfigRisk : reads risk from resolution
note for CrossRepoMisconfigRisk "Collect-then-render: sentinel recorded at resolution time, emitted only at validation-fail boundary"
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A(["apm install (user entry)"])
B["_resolve_package_references()\ninstall.py:349"]
C["resolve_marketplace_plugin()\nresolver.py:683"]
D["_compute_cross_repo_misconfig_risk()\nresolver.py:248\n[Pure]"]
E{"dep_ref is None AND\ntype==github AND\ncross-repo bare-on-enterprise?"}
F["CrossRepoMisconfigRisk sentinel created"]
G["sentinel = None"]
H["MarketplacePluginResolution returned\nwith cross_repo_misconfig_risk field"]
I["_misconfig_risks[canonical] = (mp, plugin, risk)\ninstall.py:413"]
J["package validation runs\ninstall.py:521"]
K{"validation passes?"}
L["valid_outcomes.append\nno hint emitted"]
M["logger.validation_fail()\ninstall.py:535"]
N{"_misconfig_risks.get(package)\nnot None?"}
O["logger.info() hint: 'set repo to\ncorp.ghe.com/owner/repo in marketplace.json'\ninstall.py:549"]
P(["Return valid/invalid outcomes"])
A --> B
B --> C
C --> D
D --> E
E -- yes --> F
E -- no --> G
F --> H
G --> H
H --> I
H --> J
J --> K
K -- yes --> L
K -- no --> M
M --> N
N -- yes --> O
N -- no --> P
O --> P
L --> P
sequenceDiagram
actor Operator
participant install as install.py
participant resolver as resolver.py
participant logger as CommandLogger
Operator->>install: apm install (enterprise marketplace, bare cross-repo plugin)
install->>resolver: resolve_marketplace_plugin(plugin, source, ...)
resolver->>resolver: _compute_cross_repo_misconfig_risk()
resolver-->>install: MarketplacePluginResolution(cross_repo_misconfig_risk=CrossRepoMisconfigRisk(...))
install->>install: _misconfig_risks[canonical] = (mp, plugin, risk)
install->>install: validate package
install->>logger: validation_fail(package, reason)
install->>logger: info("Hint: set repo to corp.ghe.com/owner/repo in marketplace.json")
logger-->>Operator: validation failure + actionable hint displayed
Recommendation
The core sentinel design, the collect-then-render pattern, and the 540-line unit test suite are sound. Ship after resolving the logger.warning swap and the auth-expert hint-copy amendment -- both are one-liners and the 3-persona convergence on logger level makes the info/warning gap unambiguous. The two missing integration-tier tests on secure-by-default surfaces are the highest-signal follow-up and should land in the next commit or a fast-follow PR. The dependency-confusion silent-pass gap is real but is a separate hardening issue; do not hold this PR on it. The docs findings and the getattr defensive-access finding are clean-up items suitable for the same fast-follow. The CHANGELOG entry should name 'GHE enterprise marketplace' explicitly before merge.
Full per-persona findings
Python Architect
-
[recommended]
getattr()on a typed dataclass field obscures the type contract and hides rename regressions atsrc/apm_cli/commands/install.py:413
MarketplacePluginResolution.cross_repo_misconfig_riskis a declared dataclass field with a default of None, sogetattr(resolution, 'cross_repo_misconfig_risk', None)is semantically redundant. If the field is ever renamed, the getattr fallback silently returns None instead of raising AttributeError -- the risk sentinel is silently dropped and the hint never fires.
Suggested: Replace with_risk = resolution.cross_repo_misconfig_risk(direct attribute access). -
[nit]
logger.info()is wrong level for an actionable misconfiguration hint at a failure boundary atsrc/apm_cli/commands/install.py:549
Emitted immediately afterlogger.validation_fail(); info-level can be filtered in quiet-mode configurations that show warnings but suppress info.
Suggested: Change tologger.warning(...).
CLI Logging Expert
-
[recommended]
logger.info()is wrong level for an actionable misconfiguration hint atsrc/apm_cli/commands/install.py:548
The hint fires immediately afterlogger.validation_fail(). An info-level ([i] blue) message reads as ambient context rather than a recovery action. The traffic-light rule says yellow ([!]) = 'should know / must act to fix'. The operator MUST editmarketplace.jsonto recover.
Suggested: Replacelogger.info(...)withlogger.warning(...). -
[recommended] Prose 'Hint:' prefix is redundant and breaks message-writing rule Why do we need a GitHub token? #1 at
src/apm_cli/commands/install.py:549
Rule: lead with the outcome, not a label. The [!] symbol already communicates advisory intent. No otherlogger.warning/errorcall in install.py prepends a category word.
Suggested: Remove the 'Hint: ' string prefix; rewrite to lead with the misconfiguration fact: e.g. "'plugin@marketplace' is registered on 'host' but barerepo: owner/reporesolved to github.com -- if you meant the enterprise host, setrepoto 'host/owner/repo' in marketplace.json." -
[nit] Add debug-level breadcrumb on the success path for risk entries that cleared at
src/apm_cli/commands/install.py:548
If a future refactor lets_misconfig_risksbe populated AND validation passes, the hint would be silently lost. A verbose breadcrumb on the success path would surface the sentinel was attached and cleared under--verbose.
DevX UX Expert
-
[recommended]
logger.info()may suppress the hint at default verbosity atsrc/apm_cli/commands/install.py:554
IfCommandLogger.infois gated behind--verbose, an operator running a plainapm installwill see the validation failure with no explanation of how to fix it -- exactly the silent-failure UX this PR is meant to cure. The hint must be visible at default (non-verbose) verbosity. -
[nit] 'in marketplace.json' is ambiguous about which file to edit
Enterprise operators managing multiple repos may not know this means the marketplace repository'smarketplace.json, not their localapm.yml. Tighten to 'in the marketplace repository's marketplace.json' or 'in the<marketplace_name>marketplace.json'. -
[nit] 'If you meant the enterprise host' hedge softens an otherwise confident hint
Consider leading with the fix: 'To route this plugin through the enterprise host, setrepotocorp.ghe.com/platform-team/shared-toolin marketplace.json. (Skip this if you intentionally depend on the github.com copy.)'
Supply Chain Security Expert
-
[recommended] Dependency-confusion via pre-staged github.com package bypasses hint entirely at
src/apm_cli/commands/install.py:535-558
The hint is gated on validation failure. An attacker who pre-registersowner/repoongithub.comwill cause_validate_package_existsto return True (wrong package fetched, resolves cleanly),_misconfig_risksis never consulted, and the operator receives zero warning. The PR correctly addresses the honest-misconfiguration case but the dependency-confusion threat remains fully silent on the success path.
Suggested: In the validation-success branch, also check_misconfig_risks.get(package)and emit alogger.warningnoting that the package resolved cross-host to github.com and an enterprise-host alternative exists. -
[nit] Hint message surfaces enterprise hostname and internal repo path in log output at
src/apm_cli/commands/install.py:543-554
_risk.marketplace_hostand_risk.bare_repo_fieldappear verbatim in the log string. Low severity -- no tokens or secrets exposed -- but install logs containing these strings warrant internal-artifact handling.
OSS Growth Hacker
-
[recommended] Hint message should lead with the symptom, then the fix at
src/apm_cli/commands/install.py:549
Enterprise operators triaging CI failures scan for 'why did this fail' before 'what do I fix'. Prepending a single symptom clause makes the message paste-searchable in runbooks and grep-able in CI log archives. -
[recommended] 'If you meant the enterprise host' is ambiguous in multi-GHE environments at
src/apm_cli/commands/install.py:554
Large enterprises sometimes run more than one*.ghe.cominstance. The phrase 'enterprise host' does not name the host. The surrounding sentence already contains_risk.marketplace_host; inline it explicitly so copy-paste works without mental substitution. -
[nit] CHANGELOG entry for this fix should call out enterprise GHE by name at
CHANGELOG.md
If the release notes only say 'surface actionable hint for cross-repo misconfig', enterprise evaluators searching for 'GHE', 'GitHub Enterprise', or '*.ghe.com' will miss it.
Auth Expert
-
[recommended] Hint fires on any validation failure for bare cross-repo entries, not only auth-misroute failures at
src/apm_cli/commands/install.py:538-555
_needs_canonical_host_prefixreturns True for both the misconfigured-same-host path AND any legitimategithub.comcross-host dep whose canonical's first segment differs fromcorp.ghe.com. The PR's safety argument holds only when network, rate limits, andgithub.comcredentials are all healthy. A rate-limit 429, network timeout, deleted repo, or expired PAT on a legitimate cross-host dep triggers the hint and misdirects the operator.
Suggested: Add a clause: 'If this is intentionally a github.com dependency, verify your github.com credentials and that the repository is accessible.' Or gate the hint on the failure reason string containing auth-indicative text. -
[nit] Verify
dep_refat sentinel call-site is post-resolution, not pre-resolution value atsrc/apm_cli/marketplace/resolver.py:683
The docstring for_compute_cross_repo_misconfig_riskrequiresdep_refto be the already-resolved value. Confirm this is the case and add a comment to prevent future reordering. -
[nit]
suggested_qualified_repouses the full bare value including any trailing path components atsrc/apm_cli/marketplace/resolver.py:300
Ifrepocontainsowner/repo/subpath, the suggestion becomescorp.ghe.com/owner/repo/subpathwhich is not valid as arepofield value.
Suggested: Extract only the first two path segments:host + '/' + '/'.join(bare.split('/')[:2]).
Doc Writer
-
[recommended] Bare
source: owner/repoon GHE-hosted marketplaces resolves to github.com -- not documented atdocs/src/content/docs/reference/manifest-schema.md
Thesourcefield is currently documented as<owner>/<repo> (remote) or ./<path> (local)with no mention of host qualification. PR fix: hint to host-qualify cross-repo on *.ghe.com (closes #1305) #1319 adds a runtime hint telling users to qualify therepofield, but without a corresponding doc anchor, users who read the hint have nowhere to look up the correct syntax.
Suggested: Add a GHE host qualification callout under thesourcefield table in section 7.5, referencing the shorthand grammar in section 4.2. -
[recommended] No troubleshooting entry for cross-repo source host-resolution mismatch on GHE at
docs/src/content/docs/troubleshooting/install-failures.md
The install-failures troubleshooting page covers auth, network, lockfile, cache, and partial installs but not the case where a package resolves from the wrong host because the marketplacesourcefield is unqualified.
Suggested: Add a short 'Marketplace host mismatch' subsection (4-6 lines) cross-referencing manifest-schema.md section 7.5.
Test Coverage Expert
-
[recommended] Existing integration regression trap does not assert
cross_repo_misconfig_riskis populated
tests/integration/test_ghe_marketplace_install_e2e.py::TestGHEMarketplaceInstall::test_cross_repo_locks_known_silent_misroutewas written in PR fix: marketplace install auth host on *.ghe.com (closes #1285) #1292 as a before-behavior lock. PR fix: hint to host-qualify cross-repo on *.ghe.com (closes #1305) #1319 addscross_repo_misconfig_risktoMarketplacePluginResolutionbut the integration test does not assertresult.cross_repo_misconfig_risk is not None. Probe confirmed:grep 'cross_repo_misconfig_risk' tests/integration/-- zero hits. If_compute_cross_repo_misconfig_risk()silently returns None due to a future refactor, zero tests at the integration tier would fail.
Suggested: Addassert result.cross_repo_misconfig_risk is not None, 'PR #1319 should attach a risk sentinel for this config'after the existing canonical assertion.
Proof (missing at integration-with-fixtures):tests/integration/test_ghe_marketplace_install_e2e.py::TestGHEMarketplaceInstall::test_cross_repo_locks_known_silent_misroute-- proves: resolve_marketplace_plugin() attaches a CrossRepoMisconfigRisk sentinel when a *.ghe.com marketplace has a bare cross-repo dict source [secure-by-default, devx] -
[recommended] Hint-emission cascade has no integration-tier test; unit tests mock the validation boundary
No test intests/integration/exercises the full install pipeline path where a marketplace-resolved package withcross_repo_misconfig_riskfails validation and produces the hint. Probe confirmed:grep 'misconfig_risks|cross_repo_misconfig' tests/integration/-- zero hits.
Proof (missing at integration-with-fixtures):tests/integration/test_ghe_marketplace_install_e2e.py::test_cross_repo_hint_emitted_on_validation_failure-- proves: When apm install processes a *.ghe.com marketplace package with bare cross-repo dict source that fails validation, the actionable host-qualification hint reaches the user [devx, secure-by-default] -
[nit] Unit coverage for sentinel and hint boundary is thorough across 13+4 tests but cannot be run-certified in this sandbox
Proof (unknown at unit):tests/unit/marketplace/test_marketplace_resolver.py::TestCrossRepoMisconfigRisk-- proves: _compute_cross_repo_misconfig_risk returns populated risk sentinel for all triggering conditions and None for all guard conditions [secure-by-default, devx]
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
Note
🔒 Integrity filter blocked 2 items
The following items were blocked because they don't meet the GitHub integrity level.
- #1319
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved". - fix: hint to host-qualify cross-repo on *.ghe.com (closes #1305) #1319
pull_request_read: 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 | noneGenerated by PR Review Panel for issue #1319 · ● 4.7M · ◷
github-actions
Bot
removed
the
panel-review
Address PR microsoft#1319 review panel findings and the github-advanced-security ``Incomplete URL substring sanitization`` flag without changing the fix's core design. logger.info -> logger.warning ============================= The PR microsoft#1292 review panel's top-five follow-up microsoft#3 (the seed for microsoft#1305) explicitly recommended ``logger.warning`` for this exact diagnostic: "A single ``logger.warning`` (or structured install-time check) would close the UX gap... converts a silent failure into an actionable error and prevents repeat microsoft#1285-class support tickets." The original PR microsoft#1319 used ``logger.info`` on the rationale that ``CommandLogger.info`` is documented for "persistent advisory context... must survive quiet-mode suppression". The current panel's three-persona convergence (Python Architect + CLI Logging Expert + DevX UX) is that ``info`` is still visually ambient at default verbosity -- an operator scanning a red ``[x]`` line will not register an adjacent ``[i]`` as the recovery action. ``warning`` renders ``[!]`` and matches the traffic-light convention the codebase uses elsewhere. As a side benefit, ``warning`` is implemented on both ``CommandLogger`` and ``NullCommandLogger`` (``info`` is not on the latter), so the message shape is now safe against any future caller variant. Remove the ``Hint:`` prefix; the ``[!]`` symbol carries the advisory signal on its own. Inline the resolved enterprise hostname into the "registered on" clause so the test assertions can anchor on contextual prose (e.g. ``"registered on 'corp.ghe.com'"``) instead of bare hostname substrings -- which silences the CodeQL flag without weakening what the assertion verifies. Auth-expert second clause ========================= The original hint read as if the misconfigured case was the only explanation for a validation failure: "If you meant the enterprise host, set the plugin's repo field to corp.ghe.com/...". A legitimate ``github.com`` cross-host dep that fails for a transient reason (rate-limit, network, expired PAT) would read that hint and add an enterprise host prefix that breaks a working config. Append the auth-expert recommended second clause: "If this is intentionally a github.com dependency, verify your github.com credentials and that the repository is accessible." Both clauses are explicitly conditional, so neither path is misdirected. The original issue's "two intents" framing assumed validation success vs 401; this clause covers the third path (validation failure on a legitimate dep) that was not in the issue spec but is real. Integration trap + new e2e test =============================== ``test_cross_repo_locks_known_silent_misroute`` in ``tests/integration/test_ghe_marketplace_install_e2e.py`` was authored by PR microsoft#1292 specifically to give "the future microsoft#1305 fix an explicit before/after diff to assert against". The microsoft#1305 fix deliberately preserves the resolver-level routing (bare cross-repo -> github.com is correct for legitimate cross-host deps) and adds a sentinel + an install-time hint instead. Update the test's docstring to reflect this, keep the routing-preservation assertions, and add three new sentinel assertions so the metadata the install command consumes is locked at the integration tier. Add ``TestCrossRepoMisconfigHintIntegration`` with two scenarios: - ``test_cross_repo_hint_emitted_on_validation_failure``: drives the real ``_resolve_package_references`` + ``InstallLogger`` through ``capsys`` and asserts the warning-level hint contains the plugin@marketplace identity, the enterprise host anchored to its "registered on" clause, the bare repo, the host-qualified fix value, and the auth-expert second clause. - ``test_legitimate_cross_host_validation_passes_no_hint``: locks the no-pollution contract for the legitimate cross-host path that validates successfully. This matches the convention PR microsoft#1292 established with PR microsoft#1312 (microsoft#1304 closer): panel-flagged ``outcome: missing`` integration findings on secure-by-default surfaces should land an integration-tier trap, not just unit coverage. CHANGELOG ========= Add the ``[Unreleased] Fixed`` entry naming GHE enterprise marketplace explicitly so enterprise teams scanning the changelog for cross-repo misconfiguration symptoms recognize the fix on upgrade. Mirrors the PR microsoft#1292 entry style. Out of scope ============ The supply-chain finding (cross-repo bare where the same owner/repo exists on github.com with attacker-staged content) is a real dependency-confusion vector but is not the diagnostic-surface problem microsoft#1305 targets; tracked as a separate follow-up issue. The doc-writer finding referenced ``docs/manifest-schema.md`` which does not exist in this repository; documentation additions deferred to a focused docs PR.
TL;DR
For
*.ghe.commarketplaces, a cross-repo dicttype: githubsource with a barerepofield still silently mis-routes auth atgithub.com(the residue PR #1292 explicitly left out of scope). When the install subsequently fails validation, the user sees the genericnot accessible or doesn't exist -- run with --verbose for auth detailswith no pointer at the marketplace's enterprise host. This PR attaches a typed risk sentinel at resolve time and emits an actionable host-qualified hint at the install-time validation-failure boundary -- the legitimate cross-host case validates successfully and never sees the hint, so case 1 stays clean. Closes #1305.Problem
apm install shared-tool@my-marketplaceagainst a marketplace registered oncorp.ghe.com, withmarketplace.jsonentry:{ "type": "github", "repo": "platform-team/shared-tool", "path": "plugins/shared" }Today produces:
Verbose tracing reveals
Auth resolved: host=github.com(instead ofcorp.ghe.com) but the default-mode error has no pointer at the marketplace's registered host. Operators have to know to rerun with--verboseAND know to interpret the host-fallback trace.Why not in PR #1292
bare repo: owner/projon a*.ghe.commarketplace has two legitimate intents under one syntax:corp.ghe.com, pluginrepo: opensource-org/awesome-toolmeaning "a github.com open-source dep from this enterprise marketplace". Today this works correctly becauseDependencyReference.parsedefaults togithub.com, which is what the author meant.corp.ghe.com, pluginrepo: platform-team/shared-toolmeaning "the same enterprise host". Today this silently 401s with no actionable hint.A resolver-time always-on warning false-positives on the intentional case and trains operators to ignore the signal. PR #1292's review panel rejected that approach. The fix has to fire only when the misconfiguration manifests as an install failure.
Approach
Two layers, both purely additive:
Layer 1 -- resolver attaches a typed sentinel.
MarketplacePluginResolutiongains an optionalcross_repo_misconfig_risk: CrossRepoMisconfigRisk | Nonefield. A pure helper_compute_cross_repo_misconfig_riskreturns a sentinel when all of:dependency_referenceisNone(GitHub-family virtual-shorthand path; GitLab and self-managed FQDNs build a structured ref upstream and sidestep this bug entirely)plugin.sourceis a dict whose normalized type isgithub(via the existing_coerce_dict_plugin_type, coveringtype/kind/sourcesynonyms and the inferred-github fallback).gitlabandgit-subdircross-repo on enterprise marketplaces hit the same auth-routing bug but the "host-qualify with marketplace host" suggestion only matches operator intent for the GitHub family._needs_canonical_host_prefixagrees the canonical is bare and the host is GitHub-family enterprise (*.ghe.com; idempotent against already host-qualified, URL, and SSH forms)repofield is a non-emptyowner/reposhorthandThe helper is pure -- no logging, no canonical mutation. Resolver behavior is byte-identical to pre-PR; only the resolution object carries one extra optional field.
Layer 2 -- install command emits the hint at the validation-failure boundary.
_resolve_package_referencesrecords the risk in a per-call_misconfig_risksdict before validation runs. The existing_marketplace_provenancemap is only written on validation success and cannot be relied on here. When_validate_package_existsreturnsFalse(which is how the GitHub-family auth failure surfaces --AuthResolver.try_with_fallbackcollapses 401/404/network into a singleFalse, no typedAuthenticationError), the validation-fail branch emits the hint vialogger.info.The actual user-visible output, post-fix:
Why this layer, not
AuthenticationErrorThe two
raise AuthenticationErrorsites in the install pipeline (pipeline.py:235 and validation.py:497) are both gated to non-GitHub hosts -- pipeline preflight skipsis_github_hostname(host)early; validation requires theis_ado_auth_failure_signalstderr pattern. The github.com fallback path goes throughtry_with_fallbackwhich returnsFalseon failure, and the caller records(canonical, reason)intoinvalid_outcomes. DecoratingAuthenticationErrorwould be a dead-code hook for this bug -- the typed exception never fires on the github.com path. The validation-fail branch is the actual choke point for this auth-routing class.Scope and tradeoffs
repofield and network failures also trigger the hint. The wording leads with the routing fact (resolved to 'github.com') and the suggestion is conditional (If you meant the enterprise host), so the false-positive remains advisory rather than misleading. Distinguishing 401 from 404/network here would require threading HTTP status out oftry_with_fallback-- a much broader cross-cutting change with significant blast radius into every API-validation caller.type: gitlab/type: git-subdircross-repo on enterprise marketplaces hit the same auth-routing bug. Hint is deliberately not attached because "host-qualify with marketplace host" matches operator intent only for the GitHub family; for gitlab the operator presumably meant gitlab.com, not the enterprise host.owner/repohappens to exist on github.com with different content) cannot be detected without changing the routing semantics PR fix: marketplace install auth host on *.ghe.com (closes #1285) #1292 preserved. The issue acknowledges this is out of scope.Tests
TestCrossRepoMisconfigRisk(resolver, 14 cases) locks the truth table for sentinel attach / no-attach:test_cross_repo_bare_attaches_risktest_cross_repo_inferred_github_via_path_attaches_riskpathfieldtest_cross_repo_kind_github_attaches_riskkind: githubtest_cross_repo_uppercase_type_attaches_risktest_cross_repo_source_field_synonym_attaches_risksource: github(third synonym)test_cross_repo_host_qualified_no_riskrepo: corp.ghe.com/...test_cross_repo_url_form_no_riskhttps://URL formtest_cross_repo_ssh_form_no_riskgit@host:owner/repoSSH formtest_cross_repo_gitlab_type_no_risktest_cross_repo_git_subdir_type_no_risktest_in_marketplace_dict_source_no_risktest_in_marketplace_string_source_no_risktest_github_com_marketplace_cross_repo_no_risktest_compute_returns_none_on_no_slash_repo_fieldTestResolvePackageReferencesCrossRepoMisconfigHint(install, 4 cases) locks the emission contract:test_hint_emitted_on_validation_failure_with_risktest_hint_not_emitted_when_validation_passes_even_with_risktest_no_hint_when_resolution_has_no_risktest_no_hint_for_plain_owner_repo_failureowner/repofailures emit no hintBoth suites were toggle-verified: temporarily removing the resolver helper call or the install-side emission block makes the corresponding positive test fail (
assert None is not None/assert 0 == 1), proving the trap is not a mock-induced false green.Validation
End-to-end manual driver (
InstallLoggerreal, only external boundaries mocked) verifies the actual user-visible stdout across four scenarios -- #1305 textbook case, legitimate cross-host (case 1), in-marketplace path (PR #1292 domain), andtype: gitlabcross-repo. Hint fires only on scenario 1, exactly as the truth-table dictates.Full test suites:
tests/unit/marketplace/+tests/unit/commands/-- 1263 passedtests/unit/install/+tests/unit/deps/-- 895 passed, 1 skippedtests/integration/test_ghe_marketplace_install_e2e.py+tests/unit/test_install_command.py-- 107 passedNo regressions in PR #1292's regression trap (
tests/integration/test_ghe_marketplace_install_e2e.py, the #1304 closer).How to test
For a manual reproduction of the user-visible behavior change:
*.ghe.comhost:apm marketplace add --host corp.ghe.com myorg/my-marketplacemarketplace.jsonentry with{ "type": "github", "repo": "platform-team/shared-tool", "path": "plugins/shared" }apm install shared-tool@my-marketplace[i] Hint:line pointing at the host-qualified fix.Related
TestResolveMarketplacePluginGHECloud(PR fix: marketplace install auth host on *.ghe.com (closes #1285) #1292's class)