Pass reasoned 403s through unchanged in send_duo_session_input

What does this MR do and why?

Follow-up from !255535 (comment 3834408447).

send_duo_session_input rewrites every 403 from the resume route as "not waiting for input, or you cannot resume it". But find_workflow! also returns 403 for unrelated reasons — most notably forbidden_if_identity_verification_required!, whose message tells the caller to complete identity verification. Rewriting that message hid the real cause and pointed the agent at polling get_duo_session, which fails for the same reason.

Two paths on this route produce a bare 403 Forbidden: authorize!(:resume_duo_workflow, workflow) when the session isn't resumable, and find_workflow! when the read_duo_workflow check fails and identity verification isn't required. Both are access or resumability failures, so the rewritten guidance is accurate for both. API::Helpers#forbidden! appends - <reason> when given a reason, so any 403 carrying one — an identity verification failure, for example — is distinguishable from the two bare cases by exact string comparison. The aggregator now rewrites only the exact bare message and passes any reasoned 403 through unchanged, so the caller keeps the real remediation.

References

Screenshots or screen recordings

N/A — MCP tool error message only.

How to set up and validate locally

Verified end to end on GDK, steps and observed output

The two branches of error_response need a bare 403 Forbidden and a 403 carrying a reason. A bare one is easy: any session that is not resumable makes authorize!(:resume_duo_workflow, workflow) fail with no reason. A reasoned one is not, so it is forced with a one-line edit rather than by reproducing identity verification, which needs the dap_require_identity_verification flag, an unpaid Duo entitlement and an unverified user, and identity_verified? short-circuits to true unless instance identity verification is on, which additionally wants SaaS simulation.

  1. Create a token with the mcp scope. There is no checkbox for it in the UI:

    PersonalAccessToken.create!(
      user: User.find_by_username('root'), name: 'mcp', scopes: [:mcp],
      expires_at: 2.days.from_now,
      organization: Organizations::Organization.default_organization
    ).token
  2. Pick any session you own that is not resumable. No need to build one: a finished session is already non-resumable, and find_workflow! runs before the resumability check, so it only has to be yours.

    w = Ai::DuoWorkflows::Workflow.find(983)
    [w.status_name, w.resumable?, w.user_id]

    What I got: [:finished, false, 1]

  3. Call the tool. workflow_id is declared as an integer in the input schema, so pass it unquoted or the call fails schema validation before it reaches the route:

    curl -s -X POST http://gdk.test:3000/api/v4/mcp \
      -H "PRIVATE-TOKEN: $TOKEN" -H "Content-Type: application/json" \
      -d '{"jsonrpc":"2.0","method":"tools/call","id":1,"params":{"name":"send_duo_session_input",
           "arguments":{"workflow_id":983,"human_approval":true}}}' | jq '.result'

    What I got: isError: true, structuredContent.error.message is "403 Forbidden", confirming the route really answered bare, and content[0].text is the rewritten guidance:

    Session 983 is not waiting for input, or you cannot resume it. Call get_duo_session with
    workflow_id=983 to check its status; only sessions with status input_required whose last CI
    job has finished accept input.

    That is the path this MR must not break by narrowing the match.

  4. Now force a reasoned 403. In ee/lib/api/ai/duo_workflows/workflows.rb, give the existing authorize call a reason. API::Helpers#authorize! takes reason third and hands it to forbidden!, which appends - <reason>:

    authorize!(:resume_duo_workflow, workflow, 'test reason')
  5. Restart, do not rely on the reloader. Grape mounts route bodies at boot, so the edit is invisible until rails-web comes back. I confirmed this the wrong way first: the call still answered a bare 403 Forbidden from the previously mounted route.

    gdk restart rails-web
  6. Repeat the exact same call. What I got, on this branch:

    "content": [{ "type": "text", "text": "403 Forbidden - test reason" }],
    "structuredContent": { "error": { "message": "403 Forbidden - test reason" } },
    "isError": true

    The reason reaches the caller verbatim.

  7. Compare against master, keeping the same forced reason, by swapping in only the service file:

    git show origin/master:ee/app/services/mcp/tools/duo_workflows/send_duo_session_input_service.rb \
      > ee/app/services/mcp/tools/duo_workflows/send_duo_session_input_service.rb
    gdk restart rails-web

    What I got on master, same call: structuredContent.error.message still reads "403 Forbidden - test reason", but content[0].text has been overwritten with the resumability guidance from step 3. The real cause was in the payload and the text the agent reads hid it. That is the bug.

  8. Revert both files and restart:

    git checkout -- ee/app/services/mcp/tools/duo_workflows/send_duo_session_input_service.rb \
      ee/lib/api/ai/duo_workflows/workflows.rb
    gdk restart rails-web

    Re-running the call returns the step 3 output again, with structuredContent.error.message back to "403 Forbidden".

The service spec covers both branches:

bundle exec rspec ee/spec/services/mcp/tools/duo_workflows/send_duo_session_input_service_spec.rb

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Fred de Gier

Merge request reports

Loading
Loading