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
- Closes #629278 (closed)
- Original discussion: !255535 (comment 3834408447)
- Original MR: !255535 (merged)
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.
-
Create a token with the
mcpscope. 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 -
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] -
Call the tool.
workflow_idis 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.messageis"403 Forbidden", confirming the route really answered bare, andcontent[0].textis 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.
-
Now force a reasoned
403. Inee/lib/api/ai/duo_workflows/workflows.rb, give the existing authorize call a reason.API::Helpers#authorize!takesreasonthird and hands it toforbidden!, which appends- <reason>:authorize!(:resume_duo_workflow, workflow, 'test reason') -
Restart, do not rely on the reloader. Grape mounts route bodies at boot, so the edit is invisible until
rails-webcomes back. I confirmed this the wrong way first: the call still answered a bare403 Forbiddenfrom the previously mounted route.gdk restart rails-web -
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": trueThe reason reaches the caller verbatim.
-
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-webWhat I got on master, same call:
structuredContent.error.messagestill reads"403 Forbidden - test reason", butcontent[0].texthas 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. -
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-webRe-running the call returns the step 3 output again, with
structuredContent.error.messageback 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.rbMR 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.