google / google/agents-cli

Remote templates: symlinks are silently dropped since 1.4.1, breaking shared code across template variants

Abierto
#89 0 comentarios 0 reacciones 1 asignado Reclamado por @mhenc Ver en GitHub
Lenguaje dominante
Python
Estrellas
5.9k
Forks
660
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

### What happened

Since **1.4.1**, `agents-cli scaffold create --agent ` silently drops every
symlink in the template source. A template that uses an intra-repo symlink to share a
library across several template variants scaffolds "successfully" into a project that is
missing that library and fails on import.

This is a **regression**: 1.4.0 copied a symlinked directory as real files.

The guard is `copy_files.should_skip()` in
`google/agents/cli/scaffold/utils/template.py`:

```python
# Never follow symlinks from untrusted remote template sources
if path.is_symlink():
logging.warning(
f"Skipping symlink in template source (symlinks are not allowed): {path}"
)
return True
```

I understand the CWE-59 motivation (a template shipping `id_rsa -> ~/.ssh/id_rsa`). The
problem is that the check rejects *all* symlinks, including ones whose target never leaves
the cloned repository, and it does so without failing the command.

### Reproduction

A repo laid out like this, pushed to GitHub, with the template at `template/`:

```
myrepo/
├── shared/ # single source of truth
│ └── config.py
└── template/
├── agents-cli-manifest.yaml
└── app/
├── agent.py # `from .shared.config import ...`
└── shared -> ../../shared # committed as a symlink (git mode 120000)
```

```bash
agents-cli scaffold create demo \
--agent https://github.com//myrepo/template@main --prototype
```

**Expected:** `demo/app/shared/config.py` exists (this is what 1.4.0 did).
**Actual:** `demo/app/shared/` does not exist. The command prints
`✅ Success! Your agent project is ready.` and the project fails at import.

### Version bisect

I ran each release's own `copy_files` over the same template directory:

| Version | `app/shared/` in output |
|---------|-------------------------|
| 1.0.0 / 1.2.0 / 1.3.0 / 1.4.0 | present (symlink dereferenced) |
| 1.4.1 / 1.5.0 | **missing** |

The `Never follow symlinks from untrusted remote` string first appears in 1.4.1.

### Two things make this hard to notice

1. **It is a warning, not an error.** Scaffolding reports success, and the failure only
shows up later as an `ImportError` far from its cause.
2. **`local@` does not exercise the same path.** `agents-cli scaffold create --agent
local@` pre-copies the template with
`shutil.copytree(local_path, template_source_path, ignore=...)`
(`scaffold/commands/create.py`), and `copytree` defaults to `symlinks=False`, i.e. it
*dereferences*. So a template author's CI that validates with `local@` — which the docs
present as the way to test a template locally — passes, while every real user of the
published URL gets a broken project. Our CI has been green this whole time.

### Why this matters for multi-variant templates

We maintain an internal ADK template with several variants (a default ReAct agent, an A2UI
variant, a multi-agent GKE variant). All of them share one library — DI wiring for model
tiers, Secret Manager, artifact storage, logging/tracing — plus one `ruff.toml` and one
`ty.toml`.

The scaffolder only copies the directory `--agent` points at, so the only way to share
code across variants without duplicating it is an intra-repo symlink from each variant
into the shared directory. That is exactly what 1.4.1 broke. This is the same use case as
#62, which we're otherwise happy with.

The workaround is to vendor N physical copies of the shared library into the repo (one per
variant) and add a sync script plus a CI drift check. That works, but it means the "single
source of truth" only exists by convention, and every shared-code review carries N
mechanical duplicate diffs.

### Suggested fixes, in order of preference

1. **Allow a symlink whose resolved target stays inside the fetched repository.**
`Path.resolve()` the link and require it to be under the clone root, skipping it
otherwise. That preserves the CWE-59 protection completely — an escaping link is still
refused — while letting a template share code within its own repo.

One detail: the containment check needs to be against the **cloned repo root**, not the
template subdirectory. A shared library naturally sits at the repo root while the
template is a subdirectory, so the link legitimately points "up" out of the template
dir but never out of the repo.

2. **An opt-in in the template manifest**, e.g. `settings.follow_symlinks: true`, if you'd
rather the decision be explicit per template.

3. **At minimum, fail loudly.** If symlinks stay banned, please make the command error out
(or print to stderr and list every dropped path in the summary) instead of logging a
warning and reporting success. Silently producing a broken project is the worst
outcome. It would also help to have `local@` skip symlinks the same way remote fetches
do, so a template author's local validation reflects what users will actually get.

### Environment

- agents-cli 1.5.0 (and 1.4.1); last working: 1.4.0
- Linux, Python 3.13 / 3.14

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.