Skip to content

Commit bc80181

Browse files
BunsDevCopilot
andauthored
feat(api): tenant-scoped task API authentication with fail-closed token mode (#3) (#51)
Closes #3. Gate GET /api/github/tasks behind an [api] config section: 'open' keeps the unauthenticated local-development path (doctor warns), 'token' requires bearer tokens and fails closed. A service_token grants operator-wide visibility; each [[api.tenants]] token is scoped server-side to one installation id, optionally narrowed to specific repositories — the scope is enforced in the store query, not the client. Token comparison is constant-time (fixed-length digest equality), the 401 body is uniform and reveals nothing about existing data, and every read — allowed or denied — lands in a new api_audit table (schema v3) with caller, scope, action, and result. Two-installation route tests prove tenant A cannot list tenant B's tasks, missing/invalid tokens fail closed, repo narrowing holds inside an installation, and the audit trail records each read. docs/security.md and config/example.toml document the boundary; doctor validates token-mode misconfigurations and duplicate tokens. Signed-off-by: Val Alexander <bunsthedev@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6890234 commit bc80181

7 files changed

Lines changed: 521 additions & 15 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ duplicate comments.
160160
| Headless execution contract | Locked (v1) | Brief, result envelope, exit codes, and git-auth channel are pinned in [`docs/headless-contract.md`](docs/headless-contract.md) with JSON Schemas, golden fixtures, and a conformance test. |
161161
| `coven-code --headless` execution | Partial | Worker spawns headless sessions with a tokenless session brief and enforces task timeouts; result quality depends on the runtime. |
162162
| Pull request creation | Partial | Opens draft PRs from session results against the repository's resolved default/base branch. |
163-
| CovenCave task polling | Partial | Task API is served from the durable store and survives restarts; hosted control-plane auth is planned (#3). |
163+
| CovenCave task polling | Implemented | Task API served from the durable store, survives restarts, and is gated by the tenant boundary — `token` mode fails closed, tenant tokens are installation-scoped, and every read is audited (see [docs/security.md](docs/security.md)). |
164164
| Durable queue / task store | Implemented | Deliveries deduplicated by `X-GitHub-Delivery` before GitHub hears success; the SQLite `tasks` table is the queue (atomic claims, no drop path) and interrupted work is requeued at startup ([design](docs/durable-task-store.md)). |
165165
| Hosted tier | Planned | See [Hosted vs self-hosted](docs/hosted-vs-self-hosted.md). |
166166
| Familiar trust contract | Planned | See [Familiar Contract](FAMILIAR-CONTRACT.md). |

config/example.toml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,18 @@ max_retries = 2 # Retries for infra errors (exit code 2)
2424
# SQLite file — keep it on a persistent volume; parents are created at start.
2525
path = "data/coven-github.db"
2626

27+
# ── Task API auth (issue #3) ────────────────────────────────────────────────
28+
# Gate GET /api/github/tasks. "open" = unauthenticated (local development
29+
# ONLY — never expose publicly). "token" = bearer tokens required, fail
30+
# closed; the hosted posture.
31+
# [api]
32+
# mode = "token"
33+
# service_token = "GENERATE_LONG_RANDOM" # operator-wide visibility
34+
# [[api.tenants]]
35+
# token = "GENERATE_LONG_RANDOM" # one installation's visibility
36+
# installation_id = 123456
37+
# repos = ["your-org/your-repo"] # optional narrower scope
38+
2739
# ── Familiar configuration ──────────────────────────────────────────────────
2840
# Add one [[familiars]] block per familiar you want to expose as a GitHub bot.
2941

crates/config/src/lib.rs

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ pub struct Config {
1919
/// Hosted memory governance policy (issue #6). Absent section = memory off.
2020
#[serde(default)]
2121
pub memory: MemoryConfig,
22+
/// Task API authentication (issue #3). Absent section = open mode, which
23+
/// is only safe for local development.
24+
#[serde(default)]
25+
pub api: ApiConfig,
2226
}
2327

2428
/// Hosted memory governance policy (issue #6). Off by default; opting in is a
@@ -71,6 +75,40 @@ fn default_true() -> bool {
7175
true
7276
}
7377

78+
/// Task API access control. See `docs/security.md`.
79+
#[derive(Debug, Clone, Default, Deserialize, Serialize)]
80+
pub struct ApiConfig {
81+
/// `open` (unauthenticated; local development only) or `token`
82+
/// (bearer tokens required; fail closed — the hosted posture).
83+
#[serde(default)]
84+
pub mode: ApiMode,
85+
/// Operator-wide token with visibility across every installation
86+
/// (self-hosted Cave polling).
87+
pub service_token: Option<String>,
88+
/// Tenant tokens scoped to a single installation (and optionally to a
89+
/// subset of its repositories).
90+
#[serde(default)]
91+
pub tenants: Vec<TenantToken>,
92+
}
93+
94+
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize, Serialize)]
95+
#[serde(rename_all = "snake_case")]
96+
pub enum ApiMode {
97+
#[default]
98+
Open,
99+
Token,
100+
}
101+
102+
/// A bearer token granting read access to one installation's tasks.
103+
#[derive(Debug, Clone, Deserialize, Serialize)]
104+
pub struct TenantToken {
105+
pub token: String,
106+
pub installation_id: u64,
107+
/// Optional narrower scope: only these `owner/name` repositories.
108+
#[serde(default)]
109+
pub repos: Vec<String>,
110+
}
111+
74112
/// Durable store location. See `docs/durable-task-store.md`.
75113
#[derive(Debug, Clone, Deserialize, Serialize)]
76114
pub struct StorageConfig {
@@ -367,6 +405,52 @@ impl Config {
367405
));
368406
}
369407

408+
// ── Task API auth (issue #3) ────────────────────────────────────
409+
match self.api.mode {
410+
ApiMode::Open => {
411+
if self.api.service_token.is_some() || !self.api.tenants.is_empty() {
412+
out.push(Diagnostic::warning(
413+
"api.mode",
414+
"tokens are configured but api.mode is 'open' — they are ignored; set api.mode = \"token\" to enforce them.",
415+
));
416+
} else {
417+
out.push(Diagnostic::warning(
418+
"api.mode",
419+
"the task API is unauthenticated (api.mode = \"open\") — fine for local development, never expose it publicly; hosted deployments must use \"token\".",
420+
));
421+
}
422+
}
423+
ApiMode::Token => {
424+
if self.api.service_token.is_none() && self.api.tenants.is_empty() {
425+
out.push(Diagnostic::error(
426+
"api.mode",
427+
"api.mode is 'token' but no api.service_token or [[api.tenants]] tokens are configured — every task API call would fail.",
428+
));
429+
}
430+
}
431+
}
432+
let mut seen_api_tokens = std::collections::HashSet::new();
433+
for candidate in self
434+
.api
435+
.service_token
436+
.iter()
437+
.chain(self.api.tenants.iter().map(|t| &t.token))
438+
{
439+
let trimmed = candidate.trim();
440+
if trimmed.len() < 16 {
441+
out.push(Diagnostic::warning(
442+
"api.tenants[].token",
443+
"an API token is shorter than 16 characters — use a long random string.",
444+
));
445+
}
446+
if !trimmed.is_empty() && !seen_api_tokens.insert(trimmed) {
447+
out.push(Diagnostic::error(
448+
"api.tenants[].token",
449+
"duplicate API token — two scopes would be indistinguishable at the boundary.",
450+
));
451+
}
452+
}
453+
370454
// ── Review policy ───────────────────────────────────────────────
371455
let known_ids: std::collections::HashSet<&str> =
372456
self.familiars.iter().map(|f| f.id.as_str()).collect();
@@ -495,6 +579,12 @@ fn next_step_for(field: &str, _message: &str) -> &'static str {
495579
"memory.approval_required" => {
496580
"Keep memory.approval_required = true so learned facts need maintainer review, or accept the risk deliberately."
497581
}
582+
"api.mode" => {
583+
"Set api.mode = \"token\" and configure api.service_token and/or [[api.tenants]] tokens for hosted use."
584+
}
585+
"api.tenants[].token" => {
586+
"Generate long random tokens (e.g. openssl rand -hex 32) and keep each scope's token unique."
587+
}
498588
_ => "Update this config field, then rerun coven-github doctor.",
499589
}
500590
}
@@ -595,6 +685,7 @@ mod tests {
595685
review: ReviewConfig::default(),
596686
storage: StorageConfig::default(),
597687
memory: MemoryConfig::default(),
688+
api: ApiConfig::default(),
598689
}
599690
}
600691

@@ -869,6 +960,55 @@ mod tests {
869960
assert!(errs.contains(&"familiars"));
870961
}
871962

963+
#[test]
964+
fn token_mode_without_tokens_is_an_error_and_open_mode_warns() {
965+
let dir = tmpdir();
966+
let pem = write_pem(&dir);
967+
let bin = write_bin(&dir);
968+
let mut cfg = config_with(
969+
GitHubAppConfig {
970+
app_id: 123,
971+
private_key_path: pem,
972+
webhook_secret: "a-long-random-webhook-secret".into(),
973+
api_base_url: None,
974+
},
975+
WorkerConfig {
976+
concurrency: 4,
977+
coven_code_bin: bin,
978+
workspace_root: dir.clone(),
979+
timeout_secs: 600,
980+
max_retries: 2,
981+
},
982+
vec![good_familiar()],
983+
);
984+
985+
// Default open mode: a warning, not an error.
986+
let diags = cfg.check();
987+
assert!(errors(&diags).is_empty(), "diags: {diags:?}");
988+
assert!(
989+
diags
990+
.iter()
991+
.any(|d| d.field == "api.mode" && d.message.contains("unauthenticated")),
992+
"open mode must warn: {diags:?}"
993+
);
994+
995+
// Token mode with nothing to match against would deny every call.
996+
cfg.api.mode = ApiMode::Token;
997+
let diags = cfg.check();
998+
assert!(
999+
errors(&diags).contains(&"api.mode"),
1000+
"token mode without tokens must error: {diags:?}"
1001+
);
1002+
1003+
// A configured tenant token clears the error.
1004+
cfg.api.tenants = vec![TenantToken {
1005+
token: "a-long-random-api-token".into(),
1006+
installation_id: 1,
1007+
repos: vec![],
1008+
}];
1009+
assert!(errors(&cfg.check()).is_empty());
1010+
}
1011+
8721012
#[test]
8731013
fn first_run_errors_include_operator_next_steps() {
8741014
let dir = tmpdir();

crates/store/src/lib.rs

Lines changed: 93 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use std::path::Path;
1515
use std::sync::{Arc, Mutex};
1616

1717
/// Current schema version, stored in `PRAGMA user_version`.
18-
const SCHEMA_VERSION: i32 = 2;
18+
const SCHEMA_VERSION: i32 = 3;
1919

2020
/// Handle to the durable store. Cheap to clone; all clones share one writer
2121
/// connection.
@@ -341,21 +341,27 @@ impl Store {
341341

342342
/// The Cave oversight projection: every non-reply task, newest first.
343343
/// `familiar_names` maps familiar ids to display names (config-owned).
344+
/// `scope` limits visibility to one tenant (issue #3); `None` is the
345+
/// adapter's own unrestricted view.
344346
pub async fn cave_list(
345347
&self,
346348
familiar_names: HashMap<String, String>,
349+
scope: Option<ApiScope>,
347350
) -> Result<Vec<TaskListItem>> {
348351
let conn = self.conn.clone();
349352
tokio::task::spawn_blocking(move || {
350353
let conn = conn.lock().expect("store mutex poisoned");
351354
let mut stmt = conn.prepare(
352355
"SELECT id, repo, familiar_id, kind, state, result_status,
353-
branch, pr_number, check_run_url, updated_at
356+
branch, pr_number, check_run_url, updated_at,
357+
installation_id
354358
FROM tasks
355359
WHERE json_extract(kind, '$.kind') <> 'command_reply'
360+
AND (?1 IS NULL OR installation_id = ?1)
356361
ORDER BY updated_at DESC, id",
357362
)?;
358-
let rows = stmt.query_map([], |row| {
363+
let installation_filter = scope.as_ref().map(|s| s.installation_id);
364+
let rows = stmt.query_map(params![installation_filter], |row| {
359365
Ok((
360366
row.get::<_, String>(0)?,
361367
row.get::<_, String>(1)?,
@@ -367,6 +373,7 @@ impl Store {
367373
row.get::<_, Option<u64>>(7)?,
368374
row.get::<_, Option<String>>(8)?,
369375
row.get::<_, String>(9)?,
376+
row.get::<_, u64>(10)?,
370377
))
371378
})?;
372379
let mut items = Vec::new();
@@ -382,7 +389,16 @@ impl Store {
382389
pr_number,
383390
check_run_url,
384391
updated_at,
392+
_installation_id,
385393
) = row?;
394+
// Repository narrowing within the installation, when scoped.
395+
if let Some(scope) = &scope {
396+
if let Some(repos) = &scope.repos {
397+
if !repos.contains(&repo) {
398+
continue;
399+
}
400+
}
401+
}
386402
let kind: TaskKind = serde_json::from_str(&kind_json)
387403
.context("stored task kind is unreadable")?;
388404
let (issue_number, issue_title) = surface_of(&kind);
@@ -413,6 +429,62 @@ impl Store {
413429
}
414430
}
415431

432+
/// Tenant visibility limits for task API reads (issue #3).
433+
#[derive(Debug, Clone)]
434+
pub struct ApiScope {
435+
pub installation_id: u64,
436+
/// `None` = every repository in the installation; `Some` narrows further.
437+
pub repos: Option<Vec<String>>,
438+
}
439+
440+
impl Store {
441+
/// Appends a task-API read to the audit trail (issue #3).
442+
pub async fn record_api_read(
443+
&self,
444+
caller: &str,
445+
scope: &str,
446+
action: &str,
447+
result: &str,
448+
) -> Result<()> {
449+
let conn = self.conn.clone();
450+
let (caller, scope, action, result) = (
451+
caller.to_string(),
452+
scope.to_string(),
453+
action.to_string(),
454+
result.to_string(),
455+
);
456+
tokio::task::spawn_blocking(move || {
457+
let conn = conn.lock().expect("store mutex poisoned");
458+
conn.execute(
459+
"INSERT INTO api_audit (at, caller, scope, action, result)
460+
VALUES (?1, ?2, ?3, ?4, ?5)",
461+
params![now_rfc3339(), caller, scope, action, result],
462+
)?;
463+
Ok(())
464+
})
465+
.await
466+
.expect("store task panicked")
467+
}
468+
469+
/// `(caller, scope, action, result)` audit rows, oldest first.
470+
pub async fn api_audit_entries(&self) -> Result<Vec<(String, String, String, String)>> {
471+
let conn = self.conn.clone();
472+
tokio::task::spawn_blocking(move || {
473+
let conn = conn.lock().expect("store mutex poisoned");
474+
let mut stmt =
475+
conn.prepare("SELECT caller, scope, action, result FROM api_audit ORDER BY id")?;
476+
let rows = stmt
477+
.query_map([], |row| {
478+
Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?))
479+
})?
480+
.collect::<std::result::Result<Vec<_>, _>>()?;
481+
Ok(rows)
482+
})
483+
.await
484+
.expect("store task panicked")
485+
}
486+
}
487+
416488
/// Terminal transition applied by [`Store::finish`].
417489
#[derive(Debug, Clone, Default)]
418490
pub struct Terminal {
@@ -525,6 +597,22 @@ fn migrate(conn: &Connection) -> Result<()> {
525597
conn.execute_batch("ALTER TABLE tasks ADD COLUMN result_status TEXT;")
526598
.context("failed to apply schema v2")?;
527599
}
600+
if version < 3 {
601+
// v3: task API read audit (issue #3).
602+
conn.execute_batch(
603+
r#"
604+
CREATE TABLE api_audit (
605+
id INTEGER PRIMARY KEY AUTOINCREMENT,
606+
at TEXT NOT NULL,
607+
caller TEXT NOT NULL,
608+
scope TEXT NOT NULL,
609+
action TEXT NOT NULL,
610+
result TEXT NOT NULL
611+
);
612+
"#,
613+
)
614+
.context("failed to apply schema v3")?;
615+
}
528616
conn.pragma_update(None, "user_version", SCHEMA_VERSION)?;
529617
Ok(())
530618
}
@@ -870,7 +958,7 @@ mod queue_tests {
870958
.expect("finish");
871959

872960
let items = store
873-
.cave_list(HashMap::from([("cody".to_string(), "Cody".to_string())]))
961+
.cave_list(HashMap::from([("cody".to_string(), "Cody".to_string())]), None)
874962
.await
875963
.expect("list");
876964
assert_eq!(items.len(), 1);
@@ -950,7 +1038,7 @@ mod queue_tests {
9501038
enqueue(&store, "d1", &fix_task("work")).await;
9511039
enqueue(&store, "d2", &reply).await;
9521040

953-
let items = store.cave_list(HashMap::new()).await.expect("list");
1041+
let items = store.cave_list(HashMap::new(), None).await.expect("list");
9541042
assert_eq!(items.len(), 1, "adapter replies are not Cave tasks");
9551043
assert_eq!(items[0].id, "work");
9561044
assert_eq!(items[0].status, TaskListStatus::Queued);

0 commit comments

Comments
 (0)