CCExtractor / CCExtractor/ccsync

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

Abierto
#471 3 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
TypeScript
Estrellas
37
Forks
76
Merge medio
3 d 7 h
PR fusionados (30 d)
2

Descripción

## 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).

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.