Path traversal in template resolution: `local:<id>` read escape, plus `name:`-derived credential staging that leaks another tenant's .mcp.json

[AndriiPasternak31]

Summary

template_service.get_local_template() builds a filesystem path by joining a user-supplied template id onto the local-templates directory with no containment check:

https://github.com/Abilityai/trinity/blob/dev/src/backend/services/template_service.py#L1174-L1182

def get_local_template(template_id: str) -> Optional[dict]:
    """Get a single local template by `local:<name>` id."""
    if not template_id.startswith("local:"):
        return None
    name = template_id[len("local:"):]
    template_dir = _local_templates_dir() / name   # <-- `name` is caller-controlled
    if not template_dir.is_dir():
        return None
    return _build_local_template(template_dir)

_build_local_template then reads template_dir / "template.yaml" and returns fields from it (display_name, description, …).

Reachability

routers/templates.py:

@router.get("/{template_id:path}")
async def get_template(template_id: str, current_user: User = Depends(get_current_user)):

• The :path converter permits /, so .. segments are expressible in the id.
• The gate is get_current_user only — any authenticated user, any role. No owner/admin check, no require_role.

Verified behaviour

Measured against the real _local_templates_dir():

id suffix	is_dir()	resolves to
sage	True	<base>/sage (intended)
..	True	<repo>/config — outside the templates root
../agent-templates/sage	True	back inside, via traversal
../../../../etc	False	<...>/etc (didn't exist on my box)

So the join does escape the templates root. The read is then <escaped-dir>/template.yaml.

Impact

Bounded but real: an authenticated low-privilege user can probe for directories outside the templates root and, where such a directory contains a file named template.yaml that parses as a YAML mapping, have selected fields of it reflected back in the API response.

It is not arbitrary file read — the filename is fixed (template.yaml), the content must be a YAML mapping, and only specific keys are echoed. Treat it as directory-existence probing + narrow content disclosure, not full LFI.

Not a regression — pre-existing

get_local_template is byte-identical on dev and was not introduced or modified by trinity-enterprise#128 (PR-A #1835 / PR-B #1899).

It surfaced because ent#128 PR-B briefly added a second path operation on the same tainted value (a .resolve() used only to pick a trust label), which CodeQL flagged as py/path-injection (alert 260, high). That line has since been removed in #1899 (5c0f712f) — the trust label now comes from the caller, so the new sink is gone. The underlying traversal here is untouched and still open, which is why it needs its own issue rather than riding along in a credential-declaration PR.

Suggested fix

Containment check before touching the filesystem — reject the id rather than normalise it, so the failure is loud:

name = template_id[len("local:"):]
if not name or name in (".", "..") or "/" in name or "\\" in name:
    return None

A single plain path segment is by construction a direct child of the curated root. Prior art in this repo for "validate the id, don't sanitise the path": the strict ^[A-Za-z0-9._-]+$ id guard on the MCP pipelines reader (#919), cited in architecture.md as the path-traversal guard pattern.

Worth checking whether the by-name create path (services/agent_service/crud.py, which resolves local templates by directory name) shares the same shape.

Acceptance criteria

• GET /api/templates/local:.. (and local:../<anything>) returns 404, not a built entry
• GET /api/templates/local:sage still resolves normally (no regression to the curated catalog or to hidden: true by-id resolution, bug: agent template catalog surfaces internal test/canary fixtures + a broken stub as real templates #1513)
• Regression test naming this issue, covering .., ., empty, /-bearing and \-bearing ids
• The by-name create path audited for the same join

Provenance

Found while validating trinity-enterprise#128 PR-B (#1899) — CodeQL alert 260 pointed at an adjacent line; tracing the taint to its source turned up this.

[vybe]

Re-classified — P2 → P1, theme-reliability → theme-security, +complexity-medium (2026-08-01, operator call)

Retitled to match what is actually being fixed. This issue was filed describing one sink; PR #1935 found a second, independent one with a materially larger blast radius, and the original title and body undersell it — which matters because this is what the release notes and the closed-issue record will be read from.

Sink 1 — as filed

get_local_template() joins a caller-supplied local:<id> onto the templates root with no containment check. Reachable by any authenticated principal over GET /api/templates/local:<id> (the :path converter captures /). Discloses any parseable template.yaml the backend process can reach, as arbitrary YAML subtrees in a 200.

Sink 2 — found during implementation, not in this issue's body

generate_credential_files() derived a filesystem path from a template's untrusted name: field. An uploaded name: ../../data/deployed-templates/<victim> caused another tenant's credential-bearing .mcp.json to be staged into the attacker's own agent — reproduced verbatim with a planted MARKER-CROSS-TENANT-SECRET. Narrower trigger than Sink 1 (creator role, via deploy_local_agent upload), far worse impact.

Two facts made this live rather than theoretical:

1. _TemplateResolution.github_template_path has zero assignment sites — the safe if template_base_path: arm was dead code, so the vulnerable else was the only branch that ever executed.
2. name: is not a directory name — 5 shipped templates already declare a display string there, so the join was operating on author-controlled text by design.

This is cross-tenant credential disclosure, which is why the theme moves to security and the priority to P1. Not P0 only because the fix is already written and CI-green: what remains is review attention, not discovery.

Release handling — operator decision

Release-note callout at the next cut; no separate GitHub Security Advisory. Whoever cuts the release: name this explicitly, and tell operators running multi-user instances to upgrade and rotate any credentials that were staged into agents created from uploaded templates. Single-operator self-hosted installs are not exposed to Sink 2 (it requires a second principal to have uploaded a template).

Noting for the record that PRODUCT_CONTEXT.md carries a --private routing override for pre-disclosure security work that was not used here — the working repro is public in PR #1935. Not actionable now; relevant to how the next one gets filed.

Merge order

PR #1935 lands first. #1934 (trinity#1931) and #1936 (trinity#1908) both touch files it changes — tests/unit/test_local_templates_listing.py and three docs/memory/ files — and should rebase onto dev after it merges. All three are currently draft; they need to come out of draft to get review.