Skip to content

fix(toolbox): prepare Foundry access before deploy and create - #10739

Open
Travis Angevine (trangevi) wants to merge 4 commits into
trangevi/foundry-core-accessfrom
trangevi/toolbox-targeted-fix
Open

Travis Angevine (trangevi) wants to merge 4 commits into
trangevi/foundry-core-accessfrom
trangevi/toolbox-targeted-fix

Conversation

@trangevi

Copy link
Copy Markdown
Member

Stack

Layer 2 of 3, depends on #10738 (trangevi/foundry-core-access, verified base commit 143bfc5962613ca79a8b41d208f3246d3762b900). This is the issue-targeted slice of original #10736, extracted from verified immutable source commit 7aa122c50ff86fabe45add0846353231bceb1319 while preserving newer base changes. The broader consumer changes are intentionally reserved for layer 3. Original #10736 is unchanged.

Fixes: #10718

Scope

  • Prepare developer access only for managed azure.ai.toolbox deployment and standalone azd ai toolbox create, after locally checkable input validation and before remote connection resolution or mutation. This works independently of the dependent agent's predeploy hook.
  • Reuse core pkg/foundry.AccessPreparation: actual read probe first (404 means authorized), confirmed authorization denial before inherited/equivalent role inspection, conditional project-scoped Foundry User assignment, bounded post-grant readiness, and cancellation propagation.
  • Bind the selected endpoint to active-environment project ID, endpoint, and subscription before using project scope or tenant. Matching explicit endpoints participate; unrelated/endpoint-only selections do not borrow scope or tenant. Resolve credentials through the user-access tenant and principals through the resource tenant. Preserve managed deployment's existing subscription/user-tenant fallback.
  • Wire the core synchronous progress callback and sanitized warnings to the injected writer (stderr in production), keeping stdout JSON free of access diagnostics. Include actionable structured 403 guidance, focused regression coverage, README/help, and help snapshots.

The endpoint-aware adapter is retained rather than adopting #10737's context-only ensure callback: orchestration must verify the selected endpoint/scope before selecting the credential tenant. A context-only callback would hide or duplicate that binding. No factory, manager, or global access cache was introduced.

Intentionally unchanged

No core edits, agents-extension migration, connection/skill add/remove access preparation, publish access preparation, read/delete role grants, local-authoring grants, bring-your-own endpoint grants, provisioning grants, or broad architecture/contributor documentation changes. The existing local credential file is retained; no gratuitous cleanup. Pure shared policy tests remain in the core layer.

Release/dependency gate

Keep draft / do not merge or release the extension until the core helper from #10738 is released and this extension's azd dependency is bumped to that released semver. The extension still pins github.com/azure/azure-dev/cli/azd v1.37.0, which does not contain these new APIs. Validation uses an external session-artifact GOWORK containing this checkout's core and toolbox modules with GOTOOLCHAIN=go1.26.4. No workspace, local replace, pseudo-version, or invented release version is committed. Only the directly needed authorization/v2 SDK dependency and checksums are added.

Validation

All with Go 1.26.4 and the external workspace:

  • Full toolbox go test ./... -count=1 and go build passed.
  • Targeted go vet ./internal/cmd ./internal/exterrors, go fix -diff ./internal/cmd ./internal/exterrors (no diff), and golangci-lint run ./internal/cmd/... ./internal/exterrors/... passed (0 issues).
  • Repository-configured spell checks on changed production Go files and extension README passed; git diff --check passed.
  • Regression coverage includes deploy/create ordering without an agent hook, user/service-principal tenant and identity context, actual command access wiring, explicit empty flags, invalid local definitions before access, strict project/subscription binding, unrelated-endpoint and missing-scope fallback, managed/404/unknown-403/skip non-grants, read/reuse non-preparation, warning/progress writer routing and credential non-disclosure, and cancellation including a joined canceled/404 probe.
  • Existing full-suite read/delete/local-authoring tests remain passing. No live Azure scenarios executed.

Initial local failures from a blanket NO_COLOR override in the existing color-specific help test and a test listener double-close were corrected before the final clean full-suite run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
26 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@hund030

Copy link
Copy Markdown
Contributor

Rebased onto the updated #10738 base and tightening two small things in this layer:

  1. Avoid duplicate GetToolbox round-trip after access probe (toolbox_access.go):
    toolboxAccess.prepare probes client.GetToolbox(ctx, name) (200 or 404), and createToolboxFromDefinition immediately calls client.GetToolbox(ctx, name) a second time to check whether the toolbox already exists (as do the mutation commands in fix(foundry): share access preparation across agent and toolbox writes #10740). Caching the verified probe result on a single-use probedToolboxClient wrapper eliminates the back-to-back duplicate GET request without changing any command signatures.
  2. Guard expandToolboxValue type assertion in Deploy (service_target.go:178, 357):
    If cfg.Tools contains a nil entry (e.g. tools: [null]), expandToolboxValue(tool, environment).(map[string]any) panics before reaching validateToolboxDefinition / validateRawToolEntries. Using comma-ok (expanded, _ := ...) lets validateRawToolEntries return the structured tools[i] is empty validation error cleanly.

Limit access preparation to managed deployment and standalone create after local validation and before remote connection resolution. Reuse shared probe-first policy with verified project scope, user-tenant credentials, cancellation, and stderr progress.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Cache the verified GetToolbox probe response for one-time reuse by the immediate write operation.

- Guard expandToolboxValue map type assertions when a tool entry is nil.
@hund030
Zhijie Huang (hund030) force-pushed the trangevi/toolbox-targeted-fix branch from 188fcb8 to 45c279f Compare October 28, 2026 10:24
@hund030

Copy link
Copy Markdown
Contributor

Rebased onto 19791ec69 (#10738) while preserving 188fcb8b6 (fix: reject mixed toolbox tool names before access checks), and updated toolboxAccess.prepare (1047bf21b) so Probe only treats 404 as "toolbox not found (authorized)" when !foundry.IsProjectNotFound(err) (avoiding false readiness when a freshly provisioned project returns 404 Project not found).

@github-actions

Copy link
Copy Markdown

📋 Prioritization Note

Thanks for the contribution! The linked issue isn't in the current milestone yet.
Thank you for logging this issue; our team is reviewing it. If you need urgent prioritization, tag Rick Winter (@RickWinter) and Kristen Womack (@kristenwomack) to let us know.

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.

🟡 Changes recommended

Access preparation can grant unnecessary permissions before checking access, while managed deployments hide progress and administrator remediation warnings.

2 open issues
What changed in this PR

Integrates shared Foundry access preparation into toolbox deployment and standalone creation to address #10718 without relying on an agent predeploy hook. This is layer 2 of the stack and remains gated on a published core helper and dependency update.

Changes:

  • Adds endpoint/scope binding, tenant resolution, and access-preparation wiring.
  • Moves local validation before remote operations and adds regression coverage.
  • Updates access guidance, structured 403 suggestions, help text, and dependency checksums.
File Description
cli/​azd/​extensions/​azure.ai.toolboxes/​README.md Documents access behavior, validation, opt-out, and release sequencing.
cli/​azd/​extensions/​azure.ai.toolboxes/​internal/​exterrors/​errors.go Adds actionable Foundry access guidance for 403 responses.
cli/​azd/​extensions/​azure.ai.toolboxes/​internal/​cmd/​toolbox_create.go Validates definitions before access preparation and remote calls.
cli/​azd/​extensions/​azure.ai.toolboxes/​internal/​cmd/​toolbox_create_test.go Tests duplicate authored and resolved tool names.
cli/​azd/​extensions/​azure.ai.toolboxes/​internal/​cmd/​toolbox_access.go Adds the shared-access adapter and probe-result caching.
cli/​azd/​extensions/​azure.ai.toolboxes/​internal/​cmd/​toolbox_access_test.go Covers access wiring, validation, tenant binding, diagnostics, and cancellation.
cli/​azd/​extensions/​azure.ai.toolboxes/​internal/​cmd/​testdata/​TestAIHelp-root.snap Updates root help expectations for access preparation.
cli/​azd/​extensions/​azure.ai.toolboxes/​internal/​cmd/​testdata/​TestAIHelp-create.snap Updates create help expectations for access diagnostics.
cli/​azd/​extensions/​azure.ai.toolboxes/​internal/​cmd/​service_target.go Prepares managed-deployment access before connection resolution.
cli/​azd/​extensions/​azure.ai.toolboxes/​internal/​cmd/​root.go Describes automatic access preparation in root help.
cli/​azd/​extensions/​azure.ai.toolboxes/​internal/​cmd/​context.go Adds write-specific client resolution and shared tenant lookup.
cli/​azd/​extensions/​azure.ai.toolboxes/​go.sum Records authorization SDK checksums.
cli/​azd/​extensions/​azure.ai.toolboxes/​go.mod Adds the authorization SDK dependency.

Review effort: Balanced

environment: client.Environment(),
account: client.Account(),
newCredential: foundry.NewCredential,
writer: os.Stderr,

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ext-toolboxes azure.ai.toolboxes extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Foundry toolbox deployment can fail before agent predeploy grants developer access

4 participants