Skip to content

feat(files): create folder in File Manager (#37) - #898

Merged
vybe merged 2 commits into
devfrom
feature/37-create-folder-file-manager
May 21, 2026
Merged

feat(files): create folder in File Manager (#37)#898
vybe merged 2 commits into
devfrom
feature/37-create-folder-file-manager

Conversation

@dolho

@dolho dolho commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements #37 — create new directories in the agent workspace via the File Manager UI (per-agent Files tab).

Follows the existing three-surface file-op pattern (mirrors update/delete file logic exactly):

Surface Change
Agent-server POST /api/files/mkdir — workspace-confined to /home/developer, rejects edit-protected paths (.trinity/.git/credential files, parent-walked), 409 if target exists, creates intermediate parents
Backend POST /api/agents/{name}/files/mkdircreate_agent_folder_logic — access check + _is_user_writable_path deny-list (AISEC-C2/#590 boundary) + container-running guard, proxies to agent-server, propagates 403/409
Frontend "New Folder" button + modal in FilesPanel.vue; creates inside the selected directory when one is selected, else workspace root; nested paths via /. New createAgentFolder store action

Scope notes

  • FilesPanel.vue is THE file manager (mounted in AgentDetail); views/FileManager.vue is unused/unrouted (requirements.md §13.2 deprecated) — intentionally untouched.
  • CreateFolderRequest defined inline in the router, following the existing local FileUpdateRequest convention in agent_files.py.
  • No MCP tool added — Create Folder in File Manager #37 is UI-only; not an external-surface need.

Tests

tests/test_agent_files.py::TestCreateFolder — create+list, duplicate→409, protected-path→403, unknown-agent→404, unauth→401. Mirrors the existing integration-test style (self-skips on 503 when stack is down).

Verification

  • All changed Python py_compile clean (agent-server, backend service/router/init, test)
  • Frontend vite build clean (AgentDetail bundle includes FilesPanel)
  • New tests collect (5)
  • Integration tests need a live stack (down in dev env; self-skip on 503) → exercised in CI

Docs

  • requirements.md §13.1 — feature listed, dated, issue tagged
  • architecture.md — Agents endpoint table + agent-server internal API list

Related to #37

🤖 Generated with Claude Code

Adds a "New Folder" capability to the per-agent File Manager (Files tab).

Three surfaces, mirroring the existing update/delete file path:
- Agent-server: POST /api/files/mkdir — workspace-confined, rejects
  edit-protected paths (.trinity/.git/etc.), 409 if target exists,
  creates intermediate parents.
- Backend: POST /api/agents/{name}/files/mkdir → create_agent_folder_logic
  (access check + _is_user_writable_path deny-list + container-running
  guard, proxies to agent-server). Propagates 403/409 from the agent.
- Frontend: New Folder button + modal in FilesPanel.vue; creates inside
  the selected directory when one is selected, else workspace root;
  supports nested paths via "/". New createAgentFolder store action.

Tests: TestCreateFolder in tests/test_agent_files.py (create+list,
duplicate→409, protected→403, unknown agent→404, auth→401).
Docs: requirements.md §13.1 + architecture.md endpoint tables.

Verification: all changed Python py_compile clean; frontend vite build
clean; new tests collect (5). Integration tests require a live stack
(self-skip on 503) and run in CI.

Related to #37

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread docker/base-image/agent_server/routers/files.py Fixed
Comment thread docker/base-image/agent_server/routers/files.py Fixed
Comment thread docker/base-image/agent_server/routers/files.py Dismissed
Comment thread docker/base-image/agent_server/routers/files.py Dismissed
Comment thread docker/base-image/agent_server/routers/files.py Dismissed
Comment thread docker/base-image/agent_server/routers/files.py Dismissed
…ion)

CI CodeQL flagged "uncontrolled data used in path expression" (CWE-022)
on create_folder. The str(requested_path).startswith(str(allowed_base))
guard is not a recognized CodeQL barrier and is genuinely weak: it has
a sibling-prefix bypass ("/home/developer-x".startswith("/home/developer")
is True).

Replace with Path.is_relative_to() against a resolved base. .resolve()
collapses any "../" before the check, so this is both a correct
containment check and the path-traversal barrier. Verified against
traversal, sibling-prefix, and absolute-escape cases.

Scope: only the new create_folder endpoint. update_file/delete_file in
the same file use the same legacy startswith pattern but are unchanged
here (not in #37's scope, not flagged on this PR's diff) — noted for a
separate follow-up.

Related to #37

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread docker/base-image/agent_server/routers/files.py Dismissed
Comment thread docker/base-image/agent_server/routers/files.py Dismissed

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving. Clean implementation; nice CodeQL response in commit 2 (the sibling-prefix bypass fix shows you understood the finding, not just silenced it).

Two non-blocking follow-ups:

  • Update docs/memory/feature-flows/file-browser.md with the mkdir endpoint alongside the existing delete/update API sections — keeps the flow doc as the single source of truth for the file-manager surface.
  • Open a follow-up type-bug priority-p2 issue to apply the same Path.resolve() + is_relative_to() containment to update_file and delete_file in docker/base-image/agent_server/routers/files.py. The PR body acknowledges this as intentional scope-limitation; let's track the debt explicitly so it doesn't get lost.

Optional polish: two regression tests to lock in the containment fix:

  • sibling-prefix: POST .../mkdir {"path": "/home/developer-x/foo"} → 403
  • traversal: POST .../mkdir {"path": "/home/developer/../etc/foo"} → 403

@vybe
vybe merged commit 5b2c711 into dev May 21, 2026
13 checks passed
vybe pushed a commit that referenced this pull request May 25, 2026
Several integration tests were asserting pre-change behaviour:

- tests/test_agent_timeout.py — default execution_timeout_seconds was
  raised from 900 → 3600 in #665. Rename
  test_get_timeout_default_is_900 and update the expected value.

- tests/integration/test_monitoring_service.py — /health-specific
  override in monitoring_service.py treats ReadTimeout, ReadError, and
  PoolTimeout as agent liveness signals and DOES record_failure() on
  the per-agent circuit. Rename the three `_does_not_record_` tests
  to `_records_failure` and flip the assertion from 0 → 1. The
  docstrings now point at the explicit /health-specific handlers
  rather than the unrelated TRANSIENT_TRANSPORT_EXCEPTIONS contract.

- tests/test_agent_files.py — POST /api/agents/{name}/files/mkdir
  (#898) is the first test in the file that requires the agent
  container to be visible to the backend Docker SDK before the
  request, not just the row marked status=running. Add a small
  _wait_for_container_ready helper that polls /files until the
  container is visible (probe returns anything other than 404) and
  call it in the two TestCreateFolder tests that actually hit the
  endpoint (test_create_folder_and_list, test_create_folder_duplicate_returns_409).
  The other three tests in the class fail-fast before the container
  check (deny-list 403, nonexistent-agent 404, unauth 401) and don't
  need the wait.

No production code changes — tests only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants