DEV Community

weiche chiu
weiche chiu

Posted on Originally published at williamlab.dev AI-assisted

One caller, two approvers: the bypass a reader found in my approval code

The last piece ended with a correction. On the HTTP path of aine-control-plane, the field I had described as coming from the authentication layer came from an X-AINE-Actor header that nothing verifies. I opened issue #8 with a plan: first label where each actor value came from, then count the rows where the labels disagree.

On 2 October a reader, @_firelinks, replied under that post and asked me to change the order. Their comment pointed out that the same header does more than decide whose name goes into a record. It also decides who is allowed to approve. They traced the path through the code, described how one caller could clear a two-approval requirement, and proposed a test for it. They also suggested why it had probably not mattered yet: the server binds to 127.0.0.1 by default, and that stays true only until someone puts it behind a proxy for a team to share.

They were right about the order. Their comment had already named both the role path and the quorum path. What checking the code added is that reject was open the same way: before the fix, any header caller's reject would set the approval status to "rejected".

Two ways through

An approval request in aine-control-plane can carry required_roles and required_approvals. A request that needs two people with the approver role should stay pending until two such people approve it. On the reference HTTP server, both checks read from values the caller supplies.

The role. The server builds the actor's roles from an X-AINE-Roles header. decide_approval checked those roles against required_roles. A caller who sends X-AINE-Roles: approver has the role.

The quorum. An approval counts as approved once the number of distinct actor_id values among its approve decisions reaches required_approvals. The actor_id comes from X-AINE-Actor. One caller who sends two approve decisions under two different names has supplied two distinct actors.

Put together, one process could create a request that needs two approvers, then approve it twice as two invented people, and the record would show a request approved by a quorum of two.

To keep the scale honest: aine-control-plane is a reference implementation, and its SECURITY.md already said it does not provide authentication and that consumers must supply the authenticated actor before exposing it. There is no CVE. I know of no deployment where this was used against anyone. The defect is that the reference server accepted approval decisions at all from an identity it could not check, and the documentation did not say that approvals could be decided from an unverified header identity.

The fix

It went in as two pull requests, merged on 9 October.

PR #9 covers steps 1 and 2 of issue #8: the label, plus the daily attribution counts (GET /v1/audit/actor-attribution). Records now carry actor_source, and the reference HTTP transport sets it to header. A consumer that verifies identity sets it to something else, such as token.

PR #10 uses that label. ApprovalWorkflow.decide() now checks the source before it looks anything up. If the source is missing, empty, or header, it returns approval_identity_unverified and records nothing. This applies to reject as well as approve. A consumer that verifies identity in front of the core and sets the source keeps working as before.

The PR adds four tests in tests/test_approval_identity.py, and three of them go through the real HTTP server (self-grant, quorum, reject). The fourth calls the workflow directly, and it covers a missing or header source being refused and a verified source still working. One is the test @_firelinks proposed: create a request with required_approvals: 2, post two approve decisions with different X-AINE-Actor values and the approver role, and expect it to stay pending. The others cover a caller naming itself approver and reject being refused too. For this article I ran the four tests against the commit before the fix, where all four fail, and against the merge commit, where all four pass.

@_firelinks suggested a softer version: keep recording header-sourced decisions but stop counting them toward required_approvals. PR #10 refuses them instead and stores nothing. A recorded unverified decision would still be read by _status() and by audit readers that don't filter on source, and filtering those out needs a schema change. So for now it refuses at the door.

What it costs, and what is still open

The cost is direct. The reference server can no longer decide approvals by itself. Anyone running it as shipped can create approval requests and read them, but a decision needs a layer in front that verifies who is calling and sets the source. I think that is the correct place for the reference server to stop, since it was never able to answer the question the approval asks.

After the two pull requests, four things are still open.

  • Decisions recorded before PR #10 do not store their source, so the status calculation still counts them. Filtering them out needs a schema change.
  • Creating an approval request still accepts a header actor. Anyone can still ask; only deciding is gated.
  • @_firelinks also noted that nothing stops the requester from being the one who approves, which matters when a single approval is required. That check needs no token and could ship on its own. It has not shipped yet.
  • Issue #8 stays open. Its last step, joining each call's token ID against the identity provider's issuance record, waits for real token verification.

Why a pilot's stop condition depends on this

In a recent piece on choosing a first AI pilot, I listed four stop conditions. The third reads: "Bypassed approval. An output is sent or executed without human approval."

That condition assumes the approval record can be trusted to say whether a human approved. The bypass above would not trip it. The record would show an approval, with two named approvers and the required role, and every field would look correct. Someone checking the condition would find nothing to stop for.

A stop condition about approval is only as good as the system's ability to tell who approved. Before relying on one, there are two things you can check in whatever approval tool you use:

  1. Where does the approver's identity come from at the moment of decision? If the answer is a value the caller sends, the record is the caller's own account.
  2. Can one caller produce two approvals? Try it with two names from one session and see whether the request moves.

If the first answer is "from the caller" or the second test succeeds, the stop condition cannot catch a self-approved output (it still fires when there is no approval record at all), and the pilot needs a verified identity in front of approval before it needs that condition.

Thanks to @_firelinks for reading the code closely enough to find this, and for pushing on the order.

Top comments (0)