Skip to content

canonical_path is not canonical: .. segments in wire paths are never resolved, so prefix-based workspace checks on it can be bypassed #278

Open

Description

@ranas-mukminov

Summary

normalize_wire_path() returns wire paths without resolving
.. segments, and normalize_path_arguments() returns its
result as the "canonical path" that user hooks and policy
checks are told to expect as clean absolute paths. A path
string that starts with a workspace prefix can therefore
resolve outside that workspace, while a prefix test on
canonical_path still passes.

Files

  • google/antigravity/connections/connection.py,
    normalize_wire_path() (lines 47–71)
  • google/antigravity/connections/local/local_connection_config.py,
    normalize_path_arguments() (lines 82–104) and
    WIRE_PATH_ARGUMENT_KEYS (lines 66–74)

Current code, for file:// URIs:

return urllib.request.url2pathname(parsed.path)

No ..-resolution, no symlink resolution. Plain paths
without a scheme are returned unchanged. The
normalize_path_arguments() docstring says: "User hooks
and policy checks expect clean absolute paths, so we
normalize known path fields before dispatch", and it
returns "The canonical path (the first normalized path
field found)".

Reproduction (synthetic paths only, no live target)

With the verbatim function logic, a prefix test
canonical_path.startswith("/tmp/ws/") gives:

  • file:///tmp/ws/secret.txt → /tmp/ws/secret.txt,
    prefix passes, resolves inside — correct.
  • file:///tmp/ws/../../etc/passwd →
    /tmp/ws/../../etc/passwd, prefix passes, resolves to
    /etc/passwd — bypass.
  • file:///tmp/ws/%2e%2e/%2e%2e/etc/passwd → same bypass
    after percent-decoding by url2pathname.
  • Plain /tmp/ws/../../etc/passwd → returned unchanged,
    same bypass.

This matters because Decide hooks are framed in the SDK
as "policy enforcement, permission checks, and guardrails":
any custom PreToolCallDecideHook or policy predicate
that enforces workspace containment with a prefix test
on canonical_path — the natural way to use a field
advertised as normalized/canonical — is bypassable by a
model-supplied .. path.

Precise scope limit

This does NOT prove built-in workspace containment is
bypassed. Built-in containment is enforced by the closed
Go localharness binary from LocalAgentConfig.workspaces,
not by canonical_path, and that binary was not audited.
This is a canonicalization defect in the Python layer,
not a built-in sandbox escape. Symlink escape is NOT
claimed: the fix below does not resolve symlinks.

Proposed fix (same files, no refactor)

In normalize_wire_path(), apply os.path.normpath()
to the file:// result (and to plain absolute paths),
collapsing .. lexically without touching the
filesystem or requiring the file to exist. With that
change, the cases above become: /etc/passwd (prefix
check now correctly fails), /etc/passwd for the
%2e%2e case (fails, correctly), and
/tmp/ws/sub/../file.txt → /tmp/ws/file.txt
(passes, correctly); in-workspace paths are unchanged.

Full symlink containment would need Path.resolve()-style
resolution — note it as a follow-up, not part of this fix.
(Note: normalize_workspace_path() in the same file
already uses Path.resolve() for workspace paths; the
gap is specifically in the tool-argument
normalize_wire_path() / normalize_path_arguments()
path.)

Verification note

Verification above is by pure-function runs against
synthetic paths. A repo unit test for the .. cases
should be added for normalize_wire_path /
normalize_path_arguments before or at filing; a full
SDK test run was not done here, as the SDK needs its
PyPI binary for end-to-end runs.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions