Skip to content

Router: harden route_patterns.py against malformed data + close CLI/symlink/name-invariant test gaps #280

Description

@wz-gsa

QA/security/vestigial review of scripts/route_patterns.py (+ generate_index, validate_frontmatter, tests). Security path-traversal guard is solid (verified: absolute, .., and subdir-symlink escapes all rejected; yaml.safe_load everywhere; no eval/format/shell injection; trusted --request-file vs untrusted INDEX-path asymmetry correctly enforced). Scoring/tie-breaker/determinism are correct. The gaps are robustness + test coverage.

SHOULD-FIX

  1. _extract_frontmatter has no try/except (route_patterns.py:132-138). Sibling copies in generate_index/validate_frontmatter catch yaml.YAMLError; this one doesn't. One malformed on-disk SKILL.md referenced by INDEX raises ParserError out of load_candidates and DoSes routing for every request. Wrap in try/except yaml.YAMLError: return {}.

  2. build_route crashes on priority: null (route_patterns.py:388-389). .get("priority", 50) returns None when the key is present-but-null → -None TypeError. Use .get("priority") or 50.

  3. Malformed INDEX shapes raise AttributeError/ValueError (load_candidates :152-159, route_request). Bare-string entry, string routing, list-shaped frontmatter, or list-shaped top-level all crash. Guard with isinstance checks.

  4. --json always exits 0 (:509-511), even on {"error":...} / primary: null. Non-JSON path correctly returns 1. Makes --json unusable as a CI gate. Return 1 if error or no primary.

  5. apply_delegation demotes the delegator even when the delegate target is non-viable (:356-375). If A delegates to B and B is deprecated/zero-match, A is still demoted → route returns primary: null, discarding the correct answer. Only demote when the target is itself included and score > 0.

Test gaps (confirmed absent)

  1. main() / CLI / --json / --request-file entirely untested (the trusted-vs-untrusted path distinction has zero coverage).
  2. Path guard not tested with a real symlink inside a subdirectory (only a top-level link is tested).
  3. name == id == dirname invariant not tested (ties to Skills: align SKILL.md frontmatter with the Agent Skills standard (require name + description; enforce name==dirname) #277).

NITs

  • 5 near-duplicate extract_frontmatter copies (already drifted — that drift IS Epic: Repository Bootstrap #1). Consolidate.
  • assumptions key always empty (dead scaffolding) — populate or remove.
  • --explain advertised in the module docstring but not implemented (argparse errors on it). Add or delete the doc example.
  • validate_request doesn't validate collection (request-side typo silently ignored).

Reachability, determinism, engine primitives, delegation happy-path, phrase-hit guard are all well covered (73 tests pass) — no action there.

AI-assisted (OpenCode); subagent-reviewed. Requires human review.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions