[lib-audit] S2-18 agent import persists caller dict verbatim (privileged keys, no slugify) - #2818
Conversation
The JSON import endpoint (POST /api/agents/import) wrote the caller-supplied agent dict verbatim into config.yaml, allowing privileged keys (llm_key, can_read_user_memory, permitted_models, registry_canonical_id) to be injected, and stored the name unslugified (for example 'My Agent'), producing a container slug that later routes cannot address. Fix by introducing AgentImportData, a Pydantic model with an explicit field allowlist (extra='ignore') that strips operational keys, and slugifying the name via slugify_agent_name -- the same rule the create route uses (unique_agent_slug delegates to it). The create path's validate_agent_name and save_config_locked are reused rather than duplicated. RED test: test_import_strips_privileged_keys_and_slugifies_name fails on origin/dev (name persisted as 'My Agent', privileged keys present) and passes after the fix (name slugified to 'my-agent', privileged keys absent). All 11 tests in test_agent_export_import.py pass; existing import tests and round-trip continue to pass. Docs-Reviewed: the import route is not individually documented in docs/agent-coordination.md; slug derivation is already documented there (lines 555-567) and this change makes import consistent with it. The user-visible behavior change is captured in the changelog fragment.
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe agent import endpoint now filters privileged fields through an allowlist model. It slugifies imported names, persists the filtered configuration, and restores channels and groups using the slug. ChangesAgent import hardening
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Agent imports now discard privileged payload fields and store agents under canonical slugified names, with regression coverage for the persisted result. No concrete current-head merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant ImportClient
participant ImportEndpoint
participant AgentImportData
participant ConfigStore
ImportClient->>ImportEndpoint: POST /api/agents/import
ImportEndpoint->>AgentImportData: Validate and filter agent payload
AgentImportData-->>ImportEndpoint: Return allowlisted fields
ImportEndpoint->>ConfigStore: Save agent under slugified name
ConfigStore-->>ImportClient: Return imported agent
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
|
@coderabbitai full review |
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
1 participant
CARD TITLE (intent, not commit subject): [lib-audit] S2-18 agent import persists caller dict verbatim (privileged keys, no slugify)
Autonomous build of board card tsk-ztgu6w.
The JSON import endpoint (POST /api/agents/import) wrote the caller-supplied
agent dict verbatim into config.yaml, allowing privileged keys (llm_key,
can_read_user_memory, permitted_models, registry_canonical_id) to be injected,
and stored the name unslugified (for example 'My Agent'), producing a container
slug that later routes cannot address.
Fix by introducing AgentImportData, a Pydantic model with an explicit field
allowlist (extra='ignore') that strips operational keys, and slugifying the
name via slugify_agent_name -- the same rule the create route uses
(unique_agent_slug delegates to it). The create path's validate_agent_name and
save_config_locked are reused rather than duplicated.
RED test: test_import_strips_privileged_keys_and_slugifies_name fails on
origin/dev (name persisted as 'My Agent', privileged keys present) and passes
after the fix (name slugified to 'my-agent', privileged keys absent). All 11
tests in test_agent_export_import.py pass; existing import tests and round-trip
continue to pass.
Docs-Reviewed: the import route is not individually documented in
docs/agent-coordination.md; slug derivation is already documented there (lines
555-567) and this change makes import consistent with it. The user-visible
behavior change is captured in the changelog fragment.
Files:
changelog.d/tsk-ztgu6w-import-slugify.md | 3 ++
tests/test_agent_export_import.py | 43 +++++++++++++++++++++
tinyagentos/routes/agents.py | 64 +++++++++++++++++++++++++-------
3 files changed, 96 insertions(+), 14 deletions(-)
Summary by CodeRabbit