Mahiro Hirakawa

Posted on Sep 4

cast(bool, x) is a promise to the type checker. At runtime it is the identity function.

#python #typing #security #ai

In google/adk-python, the value that decides whether a tool call needs human confirmation reaches its caller through cast(bool, await ...).

typing.cast returns its second argument. That is the entire implementation. It exists so a static checker will stop complaining, and it does nothing at all when the program runs. If the awaited expression produces None, the caller receives None. The caller then tests it for truth and skips the confirmation.

That is the same ending as the coercion defect filed against openai-agents-python as issue #4845. Different route. This one arrives by declaration instead of by conversion.

Why a static analyser has nothing to say

Ask a type checker what cast(bool, x) is and it answers bool. Correctly. That is what cast is for. The programmer asserted the type and the checker took the assertion. There is no diagnostic to emit and no line to highlight.

A grep-shaped tool has a different problem: the pair is not on one line. Formatters split cast( from bool, across a newline, so a per-line pattern never sees them together. I had to look at a window of lines rather than at single lines.

What I got wrong

Two things, and the second is the embarrassing one.

I first recorded the sites as function_tool.py:206 and mcp_tool.py:474. Those lines read return bool(self._require_confirmation), which is the safe branch. The expression that matters is a few lines above each of them, spanning :202-205 and :470-473. I recorded, and reported, the location of the code that was fine.

The reason I landed there is that my tool did find those functions, and it found them for a reason I never checked. A coercion signal had matched bool(...) on the safe branch. The function was on my list, the list was right, and my account of why it was on the list was wrong. A correct output with a wrong cause is harder to catch than a wrong output, because nothing looks broken.

The same tool was also printing a banner that named two active signals while three were running. I fixed that by printing the signal set the run actually used. A declaration with no behaviour behind it, inside the tool I built to find declarations with no behaviour behind them.

What I did not check

Whether any caller in adk-python actually passes something that resolves to None. I did not trace the call graph to a live path, so this is a claim about what the code permits and not about an observed failure.

cast is not the defect. A cast over a value that has already been checked is fine and common. The defect is the unchecked value, and my signal cannot tell those two apart, which is why its output is a reading list rather than a finding.

I have not measured how common this spelling is across the ecosystem. One repository, one commit, two sites.

Repository: crates/gx-witness is the part of the project that exists because a claim about a value is not the same as evidence about it.

Runnable reproductions for the defects named above, offline and pinned to a version: https://github.com/mahirhir/unanswered-approval

Top comments (7)

Vinh Nguyen • Sep 5

There's a neighbouring spelling where the source really does contain a check and it still lands where the cast lands. assert isinstance(flag, bool) reads as evidence to a reviewer, and under python -O it isn't there. I ran the two next to each other on 3.14.6: without -O the assert version raises on None, with -O it returns "skip" for None exactly like cast(bool, None) does. Same permissive direction ahmetozel points at, and the thing deciding which of the two behaviours you get isn't in the file — it's a flag on the process that started it.

That one is worse for a reading list than the cast is, because there's nothing to read. A cast is at least visible as a claim the author can't be held to, and you can go look at it. An assert under -O looks like the check you wanted it to be, and whether it is one depends on how the container was launched.

Mahiro Hirakawa • Sep 5

Ran yours on a different interpreter to see whether it was version-specific. It is not. Python 3.12.3:

without -O    assert isinstance   None -> AssertionError
              cast(bool, ...)     None -> skip
with -O       assert isinstance   None -> skip
              cast(bool, ...)     None -> skip

So on 3.12.3 and on your 3.14.6 the assert collapses into the cast under -O, which makes it a stronger example than the one I wrote about. cast never claimed to check anything. The assert did, and it stops checking at deployment time rather than at authoring time, so nothing in the source changes and a reviewer six months later reads a check that is not present in the process actually running.

The part I would add to "a flag on the process that started it": -O is usually set far away from this code, in a Dockerfile or a PYTHONOPTIMIZE env var, by somebody who is not thinking about approval gates at all.

Vinh Nguyen • Sep 6

There is one artifact that survives that distance. The level gets stamped into the bytecode cache filename, so a module imported under -O writes m.cpython-314.opt-1.pyc next to the plain m.cpython-314.pyc. I ran the same file three ways on 3.14.6 and ended up with all three sitting side by side, plain, opt-1 and opt-2, so __pycache__ in a built image does tell you which levels that module has actually been imported under, even though nothing at the call site does.

It is weaker than it first looks though. It records what has been imported, not what is running right now, and an image built with PYTHONDONTWRITEBYTECODE or on a read-only layer will not have the file at all. Still a different class from a flag that leaves nothing behind.

Mahiro Hirakawa • Sep 6

Both halves reproduce on 3.14.4 on Windows, a different build from your 3.14.6, and your caveat is the more useful half.

$ python use.py && python -O use.py && python -OO use.py
$ ls __pycache__
m.cpython-314.opt-1.pyc
m.cpython-314.opt-2.pyc
m.cpython-314.pyc

$ rm -rf __pycache__ && PYTHONDONTWRITEBYTECODE=1 python -O use.py
$ ls __pycache__
(no such directory)

All three side by side, and the environment variable removes the evidence entirely.

The artifact is forensic, and there is a runtime answer next to it

You put it exactly right — it records what has been imported, not what is running. The thing that answers the second question is one attribute away:

$ python probe.py
sys.flags.optimize = 0 | __debug__ = True
$ python -O probe.py
sys.flags.optimize = 1 | __debug__ = False

So the pair is: __pycache__ tells you the history of an image and can be missing; sys.flags.optimize tells you the truth about the process you are in and cannot be.

Which makes the whole class refusable rather than detectable

That turns your neighbouring spelling into something a library can close. If a module's invariants are asserts, it can decline to be loaded in the mode where they evaporate:

if not __debug__:
    raise RuntimeError("this module's checks are asserts; refusing to run with -O")

$ python useguard.py
guard loaded, gate(True) = True
$ python -O useguard.py; echo $?
RuntimeError: this module's checks are asserts; refusing to run with -O
1

Five lines, and assert isinstance(flag, bool) stops being a promise the runtime can silently withdraw. It fails at import rather than at the approval, which is the difference between a deployment that will not start and a gate that quietly stops gating.

That is the shape I should have argued for in the post. I framed cast and assert as things to avoid at the call site; the better answer is that a module whose safety rests on asserts should refuse the mode that deletes them.

Ahmet Özel • Sep 4

What makes this class of bug nasty is that the failure lands on the permissive side. None is falsy, the confirmation is skipped, and the tool call proceeds - so the bug only manifests as an action nobody approved, which is the outcome least likely to raise an exception and most likely to be noticed later by a human wondering why something ran. A cast is a claim the author cannot be held to, and the place that hurts is exactly where the value crosses an await boundary and the actual provenance is two layers away. The point about formatters splitting cast( from bool, across lines is worth more attention than it usually gets - a lot of these patterns are invisible to line-based tooling and only show up if you look at an AST or a multi-line window, which is why they survive review and CI both.

Mahiro Hirakawa • Sep 5

"A cast is a claim the author cannot be held to" is a better sentence than any in my post.

One measurement that sharpens the point about which side the failure lands on. I ran the three frameworks against a spread of accidental values rather than just None:

None   ''   []   0   ->  the guarded tool RUNS
'no'                 ->  the gate holds

The truthy non-bool is the one that behaves correctly. So the mistake that most looks like a bug when you read it, returning a reason string, gets the safe answer, and the one that looks harmless, returning an empty reason string when there is no objection, gets the unsafe one. That is a large part of why it survives review.

Your point about the await boundary and the AST is the one I underweighted. Worth adding since I wrote the post: an OpenAI maintainer has closed my openai-agents issue, and the four PRs against it, on the grounds that these values are outside the declared bool contract and belong to the application rather than the SDK. That is defensible and I am not arguing with it, but it moves the question from "the framework is wrong" to "the application is where this has to be checked", which makes line-based tooling missing it more consequential, not less.

Some comments may only be visible to logged-in visitors. Sign in to view all comments.