CCExtractor / CCExtractor/ccsync

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

Offen
#471 3 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
TypeScript
Sterne
37
Forks
76
Ø Merge
3 T. 7 Std.
Gemergte PRs (30 T.)
2

Beschreibung

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

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.