fix(install): chmod o+x parent dirs so taos user can traverse to INSTALL_DIR - #724
Merged
Conversation
…ALL_DIR When the installer runs as root, INSTALL_DIR defaults to /root/tinyagentos. The taos service user cannot chdir into /root (mode 700), so systemd exits with status 200 (CHDIR failed) before the process even starts. chmod o+x on every ancestor of INSTALL_DIR grants the traversal bit only — not read/list — so the taos user can reach the install directory without exposing the parent's contents. Closes #723
📝 WalkthroughWalkthroughThe installer now ensures systemd can traverse the full install path by granting execute permissions to parent directories. After changing ownership of ChangesSystemd CHDIR Permission Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/install-server.sh`:
- Around line 1423-1433: The current loop that walks parent directories of
INSTALL_DIR and runs chmod o+x on each (_parent variable and the while loop)
grants world execute; change it to add a taos-specific traverse ACL using
setfacl -m u:taos:--x for each existing ancestor inside the same loop (use the
same _parent and directory-check logic), and detect if setfacl fails or is not
available—if so, emit an explicit warning and only then fall back to chmod o+x
as a last resort; ensure failure of setfacl does not abort the install but logs
the fallback decision clearly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d39c6e45-7c94-4af7-a41c-5bf66be167d6
📒 Files selected for processing (1)
scripts/install-server.sh
Comment on lines
+1423
to
+1433
| # Ensure every parent directory of INSTALL_DIR is traversable by the taos | ||
| # service user — without this, systemd CHDIR fails (exit 200) when the | ||
| # install lives under a restricted root like /root (mode 700). | ||
| # o+x = traverse only, not list: minimal security impact. | ||
| _parent="$(dirname "$INSTALL_DIR")" | ||
| while [[ "$_parent" != "/" && "$_parent" != "." ]]; do | ||
| if [[ -d "$_parent" ]]; then | ||
| chmod o+x "$_parent" 2>/dev/null || true | ||
| fi | ||
| _parent="$(dirname "$_parent")" | ||
| done |
There was a problem hiding this comment.
Use taos-specific traverse permission instead of world-execute on ancestors.
chmod o+x on every parent directory grants traversal to all local users, not just taos. That broadens access beyond the stated threat model and can unintentionally expose private path traversal (e.g., user home ancestors). Use ACLs per ancestor (setfacl -m u:taos:--x) and only fall back with an explicit warning if ACL tooling is unavailable.
As per coding guidelines, the PR objective explicitly requires fixing CHDIR without broadly exposing parent directories.
Suggested patch
- # o+x = traverse only, not list: minimal security impact.
+ # Grant traverse only to `taos` on ancestors (avoid world-exec broadening).
_parent="$(dirname "$INSTALL_DIR")"
while [[ "$_parent" != "/" && "$_parent" != "." ]]; do
if [[ -d "$_parent" ]]; then
- chmod o+x "$_parent" 2>/dev/null || true
+ if command -v setfacl >/dev/null 2>&1; then
+ setfacl -m u:taos:--x "$_parent" 2>/dev/null || true
+ else
+ warn "setfacl not available; falling back to chmod o+x on $_parent"
+ chmod o+x "$_parent" 2>/dev/null || true
+ fi
fi
_parent="$(dirname "$_parent")"
done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/install-server.sh` around lines 1423 - 1433, The current loop that
walks parent directories of INSTALL_DIR and runs chmod o+x on each (_parent
variable and the while loop) grants world execute; change it to add a
taos-specific traverse ACL using setfacl -m u:taos:--x for each existing
ancestor inside the same loop (use the same _parent and directory-check logic),
and detect if setfacl fails or is not available—if so, emit an explicit warning
and only then fall back to chmod o+x as a last resort; ensure failure of setfacl
does not abort the install but logs the fallback decision clearly.
jaylfc
added a commit
that referenced
this pull request
Jun 10, 2026
…#727) * feat(security): scope knowledge store to per-user ownership (#714) Add user_id scoping to knowledge items so members only see their own data. Admins retain full visibility including legacy rows (user_id=''). - Schema: add user_id TEXT NOT NULL DEFAULT '' to knowledge_items via MIGRATIONS v1 (idempotent ALTER TABLE ADD COLUMN); new idx_ki_user_id index; legacy rows keep user_id='' - KnowledgeStore: add_item accepts user_id; list_items/search_fts/ get_item accept optional user_id filter (None = no filter = admin); add list_for_user convenience wrapper - FTS path: JOIN knowledge_items + AND i.user_id = ? filters results by user; fallback LIKE path also filters; admin (None) skips filter - IngestPipeline.submit/submit_background: accept and forward user_id - Routes: all item/search endpoints require Depends(get_current_user); ingest binds user.id; list/search/get use _scope_user_id (admin=None, member=user.id); get_item existence-hides non-owner rows (404); delete enforces require_owner_or_admin pattern (403 for non-owners) - qmd /vsearch path: post-filter results by resolving each id against get_item(user_id=filter) — scoped for members, unfiltered for admin - Tests: 20 new tests in test_knowledge_ownership.py covering store-level isolation, route-level create/list/get/delete/search (keyword+semantic) for both member and admin, legacy row visibility, and 401 without auth Signed-off-by: jaylfc <jaylfc25@gmail.com> * chore(ci): add CLA Assistant Lite workflow (self-hosted) (#713) * chore(ci): add CLA Assistant Lite workflow (self-hosted signatures) Contributors sign the CLA once by commenting the agreed phrase on their PR; signatures stored as JSON in a dedicated cla-signatures branch (nothing leaves GitHub). Points at CLA.md, scoped to dev PRs (mirrors dco.yml), founder + bots allowlisted. Complements the interim DCO check. comment.body is only used in an if-expression equality (no shell interpolation). Signed-off-by: jaylfc <jaylfc25@gmail.com> * chore(ci): SHA-pin CLA action + drop unused actions:write permission Addresses security review: pin contributor-assistant/github-action to the full commit SHA for v2.6.1 (ca4a40a) — important since that repo is archived — and remove actions:write (the action only needs contents/pull-requests/statuses). dependabot already tracks the github-actions ecosystem so SHA bumps surface as PRs. Signed-off-by: jaylfc <jaylfc25@gmail.com> --------- Signed-off-by: jaylfc <jaylfc25@gmail.com> * feat(security): scope projects and tasks by user_id (#711) (#715) Add per-user ownership to the projects store and all project/task routes, matching the auth-context pattern established for the agent registry. Schema: adds user_id TEXT NOT NULL DEFAULT '' to projects (ALTER TABLE migration guarded for idempotency; legacy rows keep user_id=''). Store: create_project accepts user_id; list_for_user(user_id) added for member-scoped listing (returns [] for empty string so legacy rows are never returned to members); list_projects kept for admin all-rows view. Routes: all project and task endpoints now Depends(current_user). - create: sets user_id from authenticated caller, not request body. - list: admin sees all via list_projects; member sees own via list_for_user. - read: existence-hiding 404 for non-owners (and for legacy rows). - mutate (update/delete/archive/add_member/set_lead/remove_member): 403 via require_owner_or_admin. - tasks: scoped through parent project owner — no user_id column on tasks. All task routes check project ownership first via _get_owned_project helper. Tests: 26 new tests in tests/test_routes_project_ownership.py covering store list_for_user, require_owner_or_admin, route create/list/read/mutate scoping, admin sees all, and legacy row visibility rules. Signed-off-by: jaylfc <jaylfc25@gmail.com> * feat(registry): governance lifecycle layer — PR 1 (status + state machine + routes + audit) (#716) Add status column to agent_registry with an explicit lifecycle state machine: pending → active/rejected, active → suspended, suspended → active, any non-terminal → revoked (terminal). external-selfjoin origin born pending; taos-deployed born active. - schema: status TEXT NOT NULL DEFAULT 'active' added via idempotent _post_init (ALTER TABLE IF NOT EXISTS via PRAGMA table_info); backfills revoked_at rows to status=revoked on first open - store: set_status() enforces _VALID_TRANSITIONS, sets revoked_at on revoke, raises ValueError on bad transition, KeyError on unknown id - store: list_inactive() → [{canonical_id, status}] for all non-active; list_all(status=) and list_for_user(user_id, status=) accept optional filter - store: revoke() now also sets status=revoked for consistency - routes: POST /{id}/approve|reject|suspend|reactivate (admin-only, 403/404/409) - routes: GET /inactive (admin-only, route declared before /{id}) - routes: GET /?status= optional filter; admin sees all, member sees own - audit: every transition writes kind="governance" event to trace_store under the taos-governance slug (best-effort, non-fatal); governance kind added to VALID_KINDS and ENVELOPE_V1_SCHEMA - tests: 48 new tests covering store transitions, migration backfill, route happy paths, 404/409, member 403, /inactive shape + ordering, status filter, audit events Signed-off-by: jaylfc <jaylfc25@gmail.com> * chore(ci): remove interim DCO check now that CLA gate is live (#717) The CLA Assistant workflow (#713) is now the contributor gate, so the interim DCO sign-off check is retired. Updates CONTRIBUTING.md to point contributors at the CLA sign-by-comment flow instead of git commit -s. 'dco' is not a required status check on any branch protection, so removing the workflow is clean. Signed-off-by: jaylfc <jaylfc25@gmail.com> * fix(security): atomic registry lifecycle transitions + rejected->revoked (#718) Addresses CodeRabbit review on the governance lifecycle (#716): - add missing rejected->revoked transition (any non-terminal can be revoked) - make set_status() atomic: conditional UPDATE WHERE status=before_status + rowcount check, so two concurrent transitions can't both win a read/validate/write race; also guarantees the audited before_status is accurate - make revoke() atomic: UPDATE WHERE revoked_at IS NULL - tighten the /inactive route-ordering test to require 200 (admin), not 200|403 - add rejected->revoked regression test Signed-off-by: jaylfc <jaylfc25@gmail.com> * feat(trust): external-agent consent loop backend (Phase 1) Add the request→notify→accept→mint→grant backend for external agents to self-onboard via a human-approved handshake rather than a config file. New modules: - tinyagentos/auth_requests_store.py: AuthRequestsStore (aiosqlite) with create / get / list_pending / count_pending_for and atomic set_decision (conditional UPDATE WHERE status='pending' — returns None on conflict) - tinyagentos/agent_grants_store.py: AgentGrantsStore with add_grant / list_grants / list_active_grants; tier='once' for Phase 1 - tinyagentos/routes/agent_auth_requests.py: five endpoints — POST /api/agents/auth-requests (exempt, no auth) GET /api/agents/auth-requests/{id} (exempt, opaque-id cap) POST /api/agents/auth-requests/{id}/approve (admin only) POST /api/agents/auth-requests/{id}/deny (admin only) GET /api/agents/auth-requests (admin only, list pending) Wiring: - app.py: AuthRequestsStore + AgentGrantsStore created, init'd in lifespan, closed on shutdown, and set eagerly on app.state - routes/__init__.py: router registered before /api/agents/{name} to avoid path-param capture - auth_middleware.py: method-sensitive _is_exempt() replaces the simple path-in-set check — exempts POST /api/agents/auth-requests (create) and GET /api/agents/auth-requests/{id} (status poll) while keeping approve / deny / list admin-gated; token only returned on status==accepted Approve flow: registry.register(origin='external-selfjoin') → mint_registry_token → AgentGrantsStore.add_grant per scope + RelationshipManager.set_permission edge → AuthRequestsStore.set_decision(accepted, canonical_id, token, granted_scopes) Tests (tests/test_auth_requests.py): 20 cases covering store atomicity, route auth gates, approve/deny/409/429 paths, token visibility, and grants. Signed-off-by: jaylfc <jaylfc25@gmail.com> * fix(security): consent-approve activates the agent (pending->active) External-selfjoin agents are born 'pending' (governance lifecycle); the consent-loop approve now transitions the minted agent to 'active' after register+grant, so an approved agent is not left in the bus inactive/revocation feed (which would make @taOSmd's gate reject it). Adds a regression assertion. Signed-off-by: jaylfc <jaylfc25@gmail.com> * fix(consent): per-request lock prevents TOCTOU race + reject unsupported ?status Two concurrent approvals of the same auth-request could both pass the pending-status check, each register a separate registry entry and write duplicate grants, then one would win set_decision and the other return 409 with orphaned side-effects. Fix: serialise approve calls per request_id with an asyncio.Lock stored on app.state._approve_locks. Also: list endpoint now returns 400 for any status value other than 'pending' (Phase 1 only supports the pending feed), and removes the misleading 'pass status=all' hint from the docstring. auth_requests_store.py: remove # type: ignore on create(); raise RuntimeError if get() returns None immediately after insert (invariant violation rather than silent type lie). Tests: +2 (concurrent approve, unsupported status=400). * feat(consent): add GET /api/agents/registry/grants feed for @taOSmd enforcement Adds the active grant feed @taOSmd polls to enforce bus access. Admin-only, mirrors the /revoked and /inactive patterns. Optional ?canonical_id= filter for per-agent queries. Returns {grants: [{canonical_id, scope, tier, project_id, granted_at, expires_at}]}. Phase 1: all grants are non-expiring so the full list is always active. Tests: +3 (feed populated after approve, 403 for non-admin, canonical_id filter). * fix(memory): user-scope search pins a dedicated taOS index (not qmd's shared default) * fix(memory): user-scope search pins a dedicated taOS index, not qmd's shared default memory.py user/default scope returned None -> qmd omitted dbPath -> used qmd's DEFAULT collection, which on a shared serve can belong to another framework (on the Pi it's openclaw's workspace index). So taOS user-scope /search+/vsearch queried/returned foreign data (and hit the openclaw index's 1024-dim mismatch). Now user-scope pins an explicit taOS-owned dbPath (data/user-qmd-index/index.sqlite); an empty index returns no results, never another framework's data. Per-agent scope unchanged. Found via @taOSmd. Regression tests added. Signed-off-by: jaylfc <jaylfc25@gmail.com> * refactor(memory): remove redundant db_path guards + fix stale docstring _agent_db_path always returns a non-empty str — the if db_path guards in browse, collections and delete-chunk routes were always true. Inline the call directly. Also update the module-level docstring which still referenced the old shared qmd default path. --------- Signed-off-by: jaylfc <jaylfc25@gmail.com> * feat(observability): Phase 3 Traces tab — per-agent event timeline + OTel span waterfall - Add "traces" to DetailTab union in AgentDetailPanel - New AgentTracesPanel: chronological trace list from GET /api/agents/{name}/trace - Click any event row to fetch + render OTel span waterfall (GET /otel-spans?trace_id=) - Summary strip: llm_call count, total tokens, avg latency, total cost - reasoning_audit verdict badge (pass/warn/fail) wired to the most recent audit event - Debug toggle reveals raw payload JSON on expanded rows - Polls every 10s matching the Logs tab cadence * fix(install): remove stale user-level qmd-serve.service on upgrade The April 2026 development unit (rkllama backend, port 7832) races the canonical qmd.service on reinstall/upgrade, causing EADDRINUSE and broken memory search. Migration step stops/disables/removes it. * feat(observability): Phase 4 — reasoning judge (post-hoc session audit) - New tinyagentos/otel/judge.py: ReasoningJudge fires after session_end - Skips trivial runs (needs ≥1 llm_call + ≥1 tool_call) - Calls LiteLLM :4000 with reasoning trace; parses verdict pass/warn/fail - Unwraps markdown-fenced JSON from model output - Stores reasoning_audit event (never emitted as OTel span — spec §4.7) - Model: system-wide memory model via taosmd.get_memory_model(), falls back to kilo-auto/free so no install is hard-blocked - trace_store: set_judge() + TraceStoreRegistry.set_judge() mirror set_emitter() - app.py: wire ReasoningJudge at startup alongside emitter - 19 new tests (all passing) * chore: update otel __init__ to reflect Phases 3+4 status * fix(install): chmod o+x parent dirs so taos user can traverse to INSTALL_DIR (#724) When the installer runs as root, INSTALL_DIR defaults to /root/tinyagentos. The taos service user cannot chdir into /root (mode 700), so systemd exits with status 200 (CHDIR failed) before the process even starts. chmod o+x on every ancestor of INSTALL_DIR grants the traversal bit only — not read/list — so the taos user can reach the install directory without exposing the parent's contents. Closes #723 * fix(migration): preserve NEW install's LiteLLM creds through data copy (#725) The cp -a step clobbered the NEW install's .litellm_db_url (freshly generated postgres password) with the OLD install's version, leaving LiteLLM unable to connect to the database after migration. Fix: save .litellm_db_url and .litellm_master_key before the bulk copy, then restore them so the installer-generated credentials survive intact. Also clear /tmp/taos-litellm before service start — the old root process owns that dir, and the non-root taos user cannot write to it on first boot after migration, causing the LiteLLM config step to fail silently. * feat(registry): PATCH /api/agents/registry/{id} for mutable metadata (governance PR2) (#726) Agents can now update their declared capabilities, display_name, handle, and role without re-registering. Status, framework, user_id, and timestamps remain immutable — only the owning user or an admin may patch an entry. Store: AgentRegistryStore.update() builds a targeted UPDATE so only provided (non-None) fields are written; a no-field call is a safe no-op. Route: PATCH /api/agents/registry/{canonical_id} returns the updated record (200), 404 for unknown ids, or 403 for non-owner/non-admin callers. Tests: 12 new tests covering store-level (7) and route-level (5) behaviour, including the partial-update, empty-body no-op, and 403 cases. --------- Signed-off-by: jaylfc <jaylfc25@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
INSTALL_DIRdefaults to/root/tinyagentostaosservice user cannotchdirinto/root(mode 700), causing systemd to exit with status 200 (CHDIR failed) before the process startschown -R taos:taos "$INSTALL_DIR", walk every parent dir ofINSTALL_DIRand applychmod o+x— traversal only, not read/listRoot cause
Reported by @Hermes in issue #723 (Rock 5B install, root-owned path). The
WorkingDirectory=in the systemd unit template uses the install path verbatim; systemd callschdir()as the service user beforeExecStart, which fails when/roothas mode 700.Security
o+xgrants traversal to the directory entry — not listing or reading contents./rootitself remains unexplorable by thetaosuser; it can only walk through it to reach/root/tinyagentos.Test plan
/rootis mode 700 — service starts without exit 200/home/user/tinyagentos) — fix is a no-op (parent dirs already world-traversable), no regressionCloses #723
Summary by CodeRabbit