Draft: Default local file reads to allow, not ask

What does this merge request do and why?

Pre-approves the read_only_files group when a namespace has no tool rules, so local file reads (read_file, grep, list_dir, find_files) stop prompting for approval.

No feature flag, deliberately. See the sequencing note below.

Closes #622602.

Why this is a bug and not a policy change

af137102c0c1 states the intended no-rule policy in its own commit body:

Remove that coupling in Registry.default_permission_for: preapproved (read-only) groups still default to allow, all other tools default to ask.

It replaced a constant derived from the DB column default

# 7bbac4f66c14
DEFAULT_PREAPPROVED_GROUPS = Workflow.column_defaults["pre_approved_agent_privileges"]
  .map { |id| ALL_PRIVILEGES[id][:name].to_sym }   # {1,2} => read_write_files, read_only_gitlab

with the literal %i[read_only_gitlab].

Dropping read_write_files was correct, it carries WriteFile, EditFile, Mkdir and RunTests. But read_only_files (privilege 8) is a read-only group and was never in the constant, because local reads were reachable through read_write_files and privilege 8 is not in the column default. Removing the write group left no read-file group behind.

So local reads have defaulted to ask since 22 July, against that commit's stated intent. The -- do not re-sync note meant "do not re-derive from the column", not "local reads should be Ask".

Why nobody noticed until now

The workflow row's column default keeps read_write_files pre-approved in DWS regardless of what policy says, and that group contains the read tools on the DWS side, so the gap is invisible. ai-assist!6491 made DWS clamp the row-derived pre-approval set to the JWT allow-list, which is correct and closes #608968. That is what turned a latent policy gap into a prompt on every file read.

That clamp is being reverted in ai-assist!6643, so the symptom goes away, but the defect here does not. It returns the moment the clamp does.

Sequencing, and why there is no feature flag

This should merge before the clamp returns. Once it has, re-landing the clamp is a no-op for users, because read_file will already be in the allow-list.

I built this behind a flag first and then removed it, on the reasoning that:

  • With nothing enforcing the allow-list, enabling a flag would change only the settings UI and the claim contents. It would gate an unobservable change, so there is no rollout signal to read.
  • It would make correct behaviour depend on two conditions instead of one. Anyone re-landing the clamp while the flag was off, or partway through rollout, would reproduce these prompts per namespace and silently.
  • Policy and behaviour currently disagree: the registry says local reads are Ask while DWS pre-approves them anyway. Landing unflagged makes them agree everywhere. Landing flagged preserves the disagreement wherever the flag has not reached.

The window while nothing enforces the allow-list is the safest one there will be to correct this default. Happy to be overruled if reviewers would rather have the flag.

This does not reopen #608968

default_permission_for is consulted only when no explicit rule exists. An admin who sets read_only_files to Ask or Deny still gets it enforced, because resolve_group returns :allow only when every governed tool resolves allow, so one non-allow value pulls the group back to :ask.

namespace state before after
no rules local reads prompt local reads silent
explicit read_only_files Ask prompt prompt
explicit read_only_files Deny tool unavailable tool unavailable
read_write_files / run_commands / use_git / read_write_gitlab, no rules prompt prompt

Writes, destroys and command execution are untouched.

Behaviour changes worth calling out

Settings UI. The UI now shows allow as the effective default for local read tools, via ToolRulesResolver#effective_default. That is the visible half of this change and what an admin notices first.

Partially configured groups: UI and enforcement disagree. If read_file is set to Ask and the other four tools in the group are left unconfigured, ToolRulesResolver reports each tool's own default and so renders grep and list_dir as Allow, while resolve_group demotes the whole group to :ask and the user is still prompted for them.

Enforcement is more restrictive than the display, so this is not a hole, but the settings page misreports. The same flaw already exists for read_only_gitlab (set list_issues to Ask and the identical thing happens), though to be accurate this MR does newly introduce it for read_only_files, where previously both sides said ask. Not fixed here because the fix belongs in the resolver and affects both groups. Pinned by a spec so the behaviour is recorded rather than assumed.

How to set up and validate locally

  1. Pick a group with no Ai::ToolRule rows.
  2. Start a Duo Agent Platform session against a project in it and ask the agent to read a file.
  3. Before this change the approval prompt appears; after it, the read runs silently.
  4. Add an explicit rule, read_file to Ask, on the same group and repeat. The prompt must come back.
  5. Ask the agent to run a command. The prompt must appear either way.

Merge request checklist

  • Tests added or updated for the changed behaviour
  • Documentation added/updated, if needed
  • Reviewed by the DAP governance owners before merge

Test results

264 examples, 0 failures, 1 pending
  ee/spec/lib/ai/tool_rules/registry_spec.rb
  ee/spec/services/ai/tool_rules/resolution_service_spec.rb
  ee/spec/graphql/resolvers/ai/tool_rules_resolver_spec.rb
  ee/spec/services/ai/duo_workflows/create_workflow_service_spec.rb
  ee/spec/services/ai/duo_workflows/update_agent_privileges_service_spec.rb

RuboCop clean.

New coverage:

  • the group default is allow, and the group reaches pre_approved_agent_privileges
  • an explicit read_only_files Ask withholds pre-approval from the group
  • the same Ask also withholds it from the group's unconfigured tools, pinning the display mismatch described above
  • the local surface, which ide and cli sessions resolve against, pre-approves reads too

I checked the two safety examples are not vacuous by letting a permissive fallback override an explicit Ask in resolve_group; both fail, and pass again once reverted.

Open questions for reviewers

  1. Do you want a feature flag after all? My reasoning for dropping it is above, but it is a security-relevant default and I would rather be overruled than assume.
  2. Separately from this MR: the JWT claim minting at ee/lib/api/ai/duo_workflows/workflows.rb:728 is gated only on gitlab_duo_governance_settings, while CreateWorkflowService#clamp_client_privileges is additionally gated on duo_workflow_local_tool_governance (wip, default_enabled: false). So DWS enforces local governance while Rails deliberately does not yet. Worth its own issue, and it is the reason this surfaced as abruptly as it did. Details in #622602.
  • #622602, the issue
  • #608968, the bypass !6491 (merged) fixed
  • #606359, ruled out as a cause, see the issue
  • #613544, duo_workflow_local_tool_governance rollout
Edited by Abhimanyu Singh

Merge request reports

Loading
Loading