Skip to content

fix(scanner): reject spoofed wardline decorators#26

Closed
tachyon-beep wants to merge 1 commit into
mainfrom
codex/fix-vulnerability-in-default-analyzer
Closed

fix(scanner): reject spoofed wardline decorators#26
tachyon-beep wants to merge 1 commit into
mainfrom
codex/fix-vulnerability-in-default-analyzer

Conversation

@tachyon-beep

Copy link
Copy Markdown
Collaborator

Motivation

  • The default decorator-based taint seeding accepted any FQN under wardline.decorators.* and could be spoofed by a scanned project that defined a local wardline.decorators package, causing unsafe functions to be anchored as trusted and producing false-green results.
  • The change hardens the analyzer so builtin Wardline marker decorators are only trusted when they unambiguously refer to Wardline's real exports and never when the scanned project may shadow the wardline package.

Description

  • Restrict builtin decorator matching in DecoratorTaintSourceProvider to exact known exports (wardline.decorators.<name> and wardline.decorators.trust.<name>) and reject prefix+final-segment matches for builtins; custom grammar markers retain the documented prefix+name behavior.
  • Add project awareness to SeedContext via project_modules: frozenset[str] and make the provider fail-closed if the scanned project defines a wardline module/package that could shadow the real marker package.
  • Thread a shadow-aware provider fingerprint through the parse stage (run_parse_project_stage) into analyzer resolution so summary-cache keys include whether wardline is shadowed, preventing unsafe cache reuse across shadowed/unshadowed scan roots (fingerprint_for_project).
  • Add unit tests covering nested/spoofed decorator paths, project-local wardline.decorators shadowing, and the implementation-module export form, and update pipeline tests to assert provider-fingerprint propagation.

Testing

  • Ran static/typing checks and compilation: python -m compileall (checked files), PYTHONPATH=src python -m ruff check, and PYTHONPATH=src python -m mypy all passed.
  • Exercised the provider and pipeline unit tests: PYTHONPATH=src python -m pytest tests/unit/scanner/taint/test_decorator_provider.py -q -k 'not star_imported_trust_boundary_fires_end_to_end' passed (29 passed, 1 deselected) and targeted pipeline/provider tests passed (tests/unit/scanner/test_pipeline.py, tests/unit/scanner/taint/test_provider_seedcontext.py, and several focused provider tests all passed).
  • One full end-to-end provider test (test_star_imported_trust_boundary_fires_end_to_end) still fails in this environment only because PyYAML (wardline[scanner] extra) is not installed; this is a pre-existing dependency-related limitation and not caused by the changes.
  • All new added tests (spoof/shadowing/implementation-path) ran and passed under the constrained test runs above.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 017f82c7a7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +84 to +86
module := module_dotted_name(
path.relative_to(root).as_posix() if path.is_relative_to(root) else path.as_posix()
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Account for configured source roots when detecting shadows

When a project uses a non-src source root such as source_roots: ["lib"], a local lib/wardline/decorators/__init__.py is discovered as module lib.wardline.decorators here, so _project_shadows_wardline() returns false and from wardline.decorators import trusted in lib/app.py is still anchored as Wardline's real @trusted. That leaves the spoofing false-green this change is meant to close for any configured import root other than ./src.

Useful? React with 👍 / 👎.

Comment on lines +80 to +82
project_modules = frozenset(
module
for path in stage_input.files

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Detect excluded shadow packages before trusting decorators

Because project_modules is derived only from stage_input.files, a repo can hide a local wardline/decorators/__init__.py with an exclude pattern (or otherwise omit it from discovery) while app.py is still scanned. At runtime from wardline.decorators import trusted in app.py resolves to that local package, but the scanner never sees the shadow here and still anchors @trusted as Wardline's builtin marker, preserving the spoofing false-green this patch is intended to remove.

Useful? React with 👍 / 👎.

for bt in self._boundary_types:
if last != bt.canonical_name or not fqn.startswith(bt.module_prefix + "."):
if bt.builtin:
if wardline_shadowed or not _is_builtin_decorator_fqn(fqn, bt.canonical_name, bt.module_prefix):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require builtin decorators to be import-proven

In a scanned module that defines a local object named wardline (for example a class or namespace with decorators.trusted) and then uses @wardline.decorators.trusted, there is no project module shadow to trip wardline_shadowed, but _resolve_decorator_fqn() still returns the exact builtin-looking string and this branch anchors the function as trusted. The decorator root was never proven to come from an import of Wardline's package, so local symbol spoofing still produces the false-green this hardening is trying to prevent.

Useful? React with 👍 / 👎.

tachyon-beep added a commit that referenced this pull request Jun 5, 2026
Close a HIGH-severity false-green. The decorator taint provider trusted any
FQN whose prefix was a builtin marker module and whose final segment was a
known marker name, without verifying it resolved to Wardline's real package.
A scanned project shipping its own `wardline/decorators` (or `loom_markers`)
no-op could spoof `@trusted`/`@trust_boundary`, launder untrusted data up to a
trusted tier, and suppress real taint->sink findings.

- Builtin markers now match ONLY exact exports (`P.<name>`, `P.trust.<name>`);
  nested-path spoofs are rejected. Custom grammar markers keep prefix+name.
- Fail closed for builtin markers under any marker root the scanned project
  shadows; shadow roots are derived dynamically from BUILTIN_BOUNDARY_TYPES
  (`wardline` AND `loom_markers`), not hardcoded.
- Thread a per-root shadow-aware provider fingerprint through the parse stage
  into both dirty-detection and the resolver summary cache, so a TRUSTED
  summary computed under a non-shadowed root cannot be reused under a shadowed
  one (cross-root cache poisoning).
- Document the Clarion fact-key residual: the content-hash freshness gate is a
  cross-tool contract Clarion independently recomputes, so a Wardline-private
  shadow bit cannot be folded in without a Clarion read-path change. Opt-in
  (`--clarion-url`) path, not the scan gate.

Reimplements and generalizes the fix proposed in #26 (which closed only the
`wardline` root and now conflicts with main). Full suite green (2390 passed),
ruff + mypy clean, self-scan unchanged (0 new defects).

Co-authored-by: John Morrissey <john@wardline.dev>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tachyon-beep

Copy link
Copy Markdown
Collaborator Author

Fixed ourselves in d352344 (PR #27, merged to main).

The vulnerability is real and HIGH-severity — I reproduced the false-green live on main for the wardline root: a scanned project shipping a no-op wardline/decorators (or loom_markers) package can spoof @trusted/@trust_boundary, launder untrusted data to a trusted tier, and suppress real taint→sink findings. Your diagnosis and overall approach (exact-export matching + project-shadow fail-closed + shadow-aware cache key) were correct, and #27 keeps that structure.

We wrote our own fix rather than merge this for two reasons:

  1. Under-fix — same vuln class left open. This PR hardcodes the wardline root, but loom_markers is an equally-builtin marker root (builtin=True in BUILTIN_BOUNDARY_TYPES) and a top-level, more-easily-shadowed import name. A project shipping a local loom_markers with a no-op trusted still anchored trust under your patch. fix(scanner): reject spoofed builtin trust decorators (supersedes #26) #27 derives the shadow-root set dynamically from BUILTIN_BOUNDARY_TYPES (wardline and loom_markers, plus any future builtin root) and encodes the per-root shadowed set into the provider fingerprint — so a scan shadowing only wardline and one shadowing only loom_markers don't collide on the cache key.
  2. Conflicts with main (the branch had diverged), so it couldn't merge as-is.

Also addressed beyond this PR: the Clarion taint-fact freshness key is whole-file-content-only and can't see shadow state, so the MCP explain_taint/Clarion read path could serve a stale trusted fact across a shadow change. That key is a cross-tool contract Clarion independently recomputes, so it's documented as a residual at the keying site + in the CHANGELOG rather than broken (lower impact: opt-in --clarion-url, not the scan gate).

Verification on #27: full suite 2390 passed, ruff + mypy clean, both spoofs defeated in a live repro while a real boundary still suppresses, and wardline's own self-scan is unchanged (0 new defects). Thanks for catching this — closing in favour of #27.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant