ai-gov: http method and path rules, and approval-required network access - #26051
ai-gov: http method and path rules, and approval-required network access#26051craig-osterhout wants to merge 2 commits into
Conversation
Signed-off-by: Craig Osterhout <craig.osterhout@docker.com>
✅ Deploy Preview for docsdocker ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: Craig Osterhout <craig.osterhout@docker.com>
derekmisler
left a comment
There was a problem hiding this comment.
read through the diff and cross-checked against the actual implementations in governor-lib, governor-services, sandboxes, and the two open frontends PRs (#17054 and #17125). overall the HTTP method/path docs are solid; the approval section has one critical bug to resolve before it can be accurate.
critical: PR #17054 currently breaks the approval feature entirely
the sandboxd network approval handler (user_prompts_network.go) explicitly bails out when the policy's maxScope is not ScopePersistent:
if !approvalScopeOffered(pending.Options, approval.ScopePersistent) {
// returns false, no prompt is published
return false
}PR #17054 as currently written hardcodes maxScope: 'APPROVAL_SCOPE_PER_REQUEST' in the create/edit action. with that value, approvalScopeOffered(options, ScopePersistent) is false on every request, the handler exits without publishing a prompt, and approvalPromptPublished stays false. the user message then falls through to the generic denial branch, the "Approval required for X. Review and respond with: sbx policy approval ls" message never appears, no entry shows in sbx policy approval ls, and there is nothing the developer can respond to. every gated destination is silently blocked.
the fix is maxScope: 'APPROVAL_SCOPE_PERSISTENT'. the docs describe the persistent flow throughout (stored rules, sbx policy ls --created-via approval, sbx policy rm network to remove them, sbx policy reset clears them), all of that only works when the scope is Persistent, which is what the sandboxes daemon actually requires.
HTTP rules: accurate (a few notes)
-
method list: backend
httpMethodNamesinspec.gohas all 9 (GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, CONNECT, TRACE), docs are correct that local CLI accepts all 9. the org UI (PR #17125) exposes only 7 (no CONNECT, TRACE). the org.md caveat "from the methods listed" and the concepts.md note "the methods you can select individually depend on where you configure the rule" cover this correctly. -
"any ()" in the org composer: the current
NetworkRuleComposersends an empty actions array →http:request:*wildcard (all 9 methods). the new card-based form in PR #17125 converts empty actions to[...L7_REQUEST_ACTIONS]= the 7 specific actions instead of the wildcard. the docs say "every method the composer offers" which is the accurate qualifier either way, but worth flagging: after #17125 lands, "any ()" in the org UI explicitly covers only the 7 shown methods. a sandbox making a CONNECT or TRACE request to that destination would not match the org allow rule. -
composerFormToPendingRulein PR #17125: emptypatternsarray → appends/**to the destination. the docs claim "leave it empty to match any path" is correct. -
the concepts.md layering table, the "HTTP allow only" row, and the "network deny is a floor" framing all match the engine behavior i could verify.
approval flow mechanics: accurate
- "approval required" message format matches
engine_governance.go:"Approval required for " + host + ".\n\nReview and respond with:\n sbx policy approval ls"with the support message appended after a blank line ✓ - deduplication ("repeated attempts collapse into a single entry") is handled by
userPromptManager.Requestusing a deterministic ID keyed on sandbox+action+resourceType+host ✓ - sandbox-scoped grants: the resolve call uses
WithRequestAttribute(ConditionTypeSandboxID), "another sandbox reaching the same destination asks again" ✓ - "preset rules and local adds don't satisfy approval", the resolver only checks
MechanismApprovalprovenance ✓ - "a local deny takes precedence over an approval requirement" ✓
- "dismiss" leaves no persistent rule, destination is requested again next time ✓
- "reset clears approvals", approvals live in the local policy store, reset wipes it ✓
depends on unmerged work
- all of the org UI HTTP rule section (Type dropdown, HTTP methods chips, Path patterns, All traffic) depends on PR #17125, which is open
- all of the "Require approval before access" org UI section (toggle label, placement, detail page "Required"/"Not required") depends on PR #17054, which is open and currently has the maxScope bug above
|
|
||
| ### Require approval for a network policy | ||
|
|
||
| Turning on **Require approval before access** means the destinations a network |
There was a problem hiding this comment.
issue: the frontend action that saves this toggle (PR #17054) currently hardcodes maxScope: 'APPROVAL_SCOPE_PER_REQUEST'. the sandboxd network approval handler (user_prompts_network.go) explicitly bails when the policy doesn't offer ScopePersistent:
if !approvalScopeOffered(pending.Options, approval.ScopePersistent) {
return false // no prompt published
}with PER_REQUEST scope, no entry ever appears in sbx policy approval ls, no "Approval required for X" message is shown, and gated destinations are silently blocked. the docs describe the persistent flow throughout (stored rules, --created-via approval, sbx policy reset clearing them), which only works with APPROVAL_SCOPE_PERSISTENT. the PR needs that change before this section is accurate.
| `api.github.com` rather than `https://api.github.com/repos`. A CIDR range | ||
| isn't accepted here. Use an **All traffic** rule for one. | ||
| - Under **HTTP methods**, select the methods the rule applies to from the | ||
| methods listed. Leave **any (\*)** selected to match every method the |
There was a problem hiding this comment.
nitpick (super non-blocking): the "any ()" chip in the card-based composer (PR #17125) converts to 7 explicit method actions (GET POST PUT DELETE PATCH HEAD OPTIONS), not the http:request:* wildcard. so "any ()" covers exactly the 7 methods the UI lists; a CONNECT or TRACE request to this destination won't match. the phrasing "every method the composer offers" is technically right, but a note that this differs from the CLI's --method ANY (which generates the true wildcard) might save a head-scratcher later.
Description
Document HTTP method and path rules, and approval-required network access.
HTTP method and path rules
A network rule can match specific HTTP methods and URL paths on a destination.
concepts.mdnetwork.mdlocal.md--methodand--pathhow-to, split out from the existing rule management. Troubleshooting entry for a method or path blocked on an allowed hostorganization.mdmonitoring.md--type httplisting with the method and path columns_index.md&_index.mdApproval-required network access
An organization network policy can require a developer to confirm each
destination before a sandbox reaches it.
network.mdsbx policy approvalworkfloworganization.mdconcepts.mdlocal.mdmonitoring.mdRelated issues or tickets
ENGDOCS-3373
Reviews