CCExtractor / CCExtractor/ccsync

GET /tasks is not serialized and Taskwarrior data isolation may race through shared /root/.task

未关闭
#471 3 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
TypeScript
星标
37
派生
76
平均合并
3 天 7 小时
30 天内合并 PR
2

描述

## Summary

Taskwarrior operations in the backend container are not isolated per request or per user. Helpers create a temporary directory and pass it as the process working directory, but Taskwarrior 3.1.0 still uses the shared default data dir `/root/.task`. `GET /tasks` (and some mutation validation paths) call `FetchTasksFromTaskwarrior` on the HTTP goroutine, outside `GlobalJobQueue`. That function starts by deleting `/root/.task`, so it can interrupt an in-flight add/edit/sync that uses the same database.

This is related to the isolation concern in #372, but that issue was closed as an `EditTaskInTaskwarrior` refactor. The current tree still deletes `/root/.task` and still does not set `TASKDATA` / `TASKRC` / `data.location`.

## Evidence

**1. Working directory is not Taskwarrior's data directory**

`ExecCommandInDir` only sets `cmd.Dir`. It does not set `TASKDATA`, `TASKRC`, `cmd.Env`, `--rc`, or `rc.data.location`:

```go
// backend/utils/exec_command.go
func ExecCommandInDir(dir, command string, args ...string) error {
cmd := exec.Command(command, args...)
cmd.Dir = dir
return cmd.Run()
}
```

`SetTaskwarriorConfig`, `SyncTaskwarrior`, and `ExportTasks` all invoke `task` through that helper. The backend image `WORKDIR` is `/root/`. In the running container, `TASKDATA` and `TASKRC` are unset.

**Runtime check (Taskwarrior 3.1.0 in `ccsync-backend-1`):** with cwd set to an empty temp dir (same as `ExecCommandInDir`), `task config ... rc.confirmation=off` printed `Config file /root/.taskrc modified.` The temp dir stayed empty. `/root/.task/taskchampion.sqlite3` was created. `task show data.location` returned `/root/.task`.

**2. `GET /tasks` is not on the job queue and wipes that shared dir**

`TasksHandler` calls `FetchTasksFromTaskwarrior` directly. `FetchTasksFromTaskwarrior` begins with `rm -rf /root/.task`, then creates a temp dir that Taskwarrior does not actually use for data.

The same `rm -rf /root/.task` appears in add/complete/delete/modify helpers. `AddTaskHandler` / `EditTaskHandler` / `ModifyTaskHandler` also call `FetchTasksFromTaskwarrior` on the request goroutine when validating dependencies, before `GlobalJobQueue.AddJob`.

**3. Concurrent wipe during sync fails Taskwarrior**

While `task sync` was running against `/root/.task`, a second process ran `rm -rf /root/.task` (the first step of `FetchTasksFromTaskwarrior`). Sync failed with:

```
Failed to synchronize with server: Clear all existing operations: attempt to write a readonly database: Error code 1032: Database cannot be modified because database file has moved
```

Afterward `/root/.task` was gone.

**4. Frontend makes the overlap likely**

After `/edit-task` returns `202` (job accepted, not finished), auto-sync-on-edit (default on) waits 1s and calls `GET /tasks` → `FetchTasksFromTaskwarrior`. `HomePage` also skips refetch on WebSocket `"Edit Task"` success, so that GET is the intended refresh.

## Reproduction

No application code was changed. This uses the backend container's Taskwarrior, matching what the Go helpers run.

1. `docker exec` into the backend container. Confirm `TASKDATA` / `TASKRC` are unset.
2. `mkdir` a temp dir and `cd` into it (equivalent to `cmd.Dir = tempDir`).
3. Run `task config sync.encryption_secret rc.confirmation=off` (and origin / client_id the same way `SetTaskwarriorConfig` does).
4. Observe: temp dir remains empty; `/root/.taskrc` is modified; `/root/.task/taskchampion.sqlite3` exists; `task show data.location` is `/root/.task`.
5. Start `task sync` in the background, then `rm -rf /root/.task` (equivalent to `FetchTasksFromTaskwarrior` overlapping a mutation/sync).
6. Sync fails with SQLite 1032 / “database file has moved”.

Expected: each request uses only its temp dir (or another per-request `TASKDATA`), and `GET /tasks` cannot delete another request's database.

Actual: all `task` processes share `/root/.task`, and unsynchronized fetches delete it.

A two-browser-account leak was not completed in the UI (Docker frontend `/auth/oauth` is not proxied; `GET /tasks` errors are also not logged). The isolation failure and the concurrent wipe are reproduced at the Taskwarrior/filesystem layer.

## Impact

- Any overlapping `FetchTasksFromTaskwarrior` (manual Sync, default auto-sync after edit, or depends validation on add/edit/modify) can delete the only Taskwarrior DB the container uses.
- That matches HTTP 500 `"Failed to fetch tasks at backend"` with no useful backend log line (`TasksHandler` does not log the underlying error).
- Cross-user data leak via the same sqlite file is plausible if two users' operations interleave, but that UI scenario was not demonstrated here. The confirmed impact is cross-request interference and failed/inconsistent sync on a shared `/root/.task`.

## Possible direction

- Point Taskwarrior at the per-request directory explicitly (`TASKDATA` / `TASKRC` or `rc.data.location`), rather than relying on `cmd.Dir`.
- Treat `FetchTasksFromTaskwarrior` with the same mutual exclusion as mutation jobs (queue or a lock around the Taskwarrior home), including the depends-validation fetches on add/edit/modify.
- Log the real `FetchTasksFromTaskwarrior` error on `GET /tasks` so 500s are diagnosable.

Related: #372 (isolation mentioned; current code still uses `/root/.task`), #367 (job-queue work).

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。