Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ duplicate comments.
| 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. |
| `coven-code --headless` execution | Partial | Worker spawns headless sessions with a tokenless session brief and enforces task timeouts; result quality depends on the runtime. |
| Pull request creation | Partial | Opens draft PRs from session results against the repository's resolved default/base branch. |
| CovenCave task polling | Partial | Task API is served from the durable store and survives restarts; hosted control-plane auth is planned (#3). |
| 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)). |
| 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)). |
| Hosted tier | Planned | See [Hosted vs self-hosted](docs/hosted-vs-self-hosted.md). |
| Familiar trust contract | Planned | See [Familiar Contract](FAMILIAR-CONTRACT.md). |
Expand Down
12 changes: 12 additions & 0 deletions config/example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,18 @@ max_retries = 2 # Retries for infra errors (exit code 2)
# SQLite file — keep it on a persistent volume; parents are created at start.
path = "data/coven-github.db"

# ── Task API auth (issue #3) ────────────────────────────────────────────────
# Gate GET /api/github/tasks. "open" = unauthenticated (local development
# ONLY — never expose publicly). "token" = bearer tokens required, fail
# closed; the hosted posture.
# [api]
# mode = "token"
# service_token = "GENERATE_LONG_RANDOM" # operator-wide visibility
# [[api.tenants]]
# token = "GENERATE_LONG_RANDOM" # one installation's visibility
# installation_id = 123456
# repos = ["your-org/your-repo"] # optional narrower scope

# ── Familiar configuration ──────────────────────────────────────────────────
# Add one [[familiars]] block per familiar you want to expose as a GitHub bot.

Expand Down
140 changes: 140 additions & 0 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ pub struct Config {
/// Hosted memory governance policy (issue #6). Absent section = memory off.
#[serde(default)]
pub memory: MemoryConfig,
/// Task API authentication (issue #3). Absent section = open mode, which
/// is only safe for local development.
#[serde(default)]
pub api: ApiConfig,
}

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

/// Task API access control. See `docs/security.md`.
#[derive(Debug, Clone, Default, Deserialize, Serialize)]
pub struct ApiConfig {
/// `open` (unauthenticated; local development only) or `token`
/// (bearer tokens required; fail closed — the hosted posture).
#[serde(default)]
pub mode: ApiMode,
/// Operator-wide token with visibility across every installation
/// (self-hosted Cave polling).
pub service_token: Option<String>,
/// Tenant tokens scoped to a single installation (and optionally to a
/// subset of its repositories).
#[serde(default)]
pub tenants: Vec<TenantToken>,
}

#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize, Serialize)]
#[serde(rename_all = "snake_case")]
pub enum ApiMode {
#[default]
Open,
Token,
}

/// A bearer token granting read access to one installation's tasks.
#[derive(Debug, Clone, Deserialize, Serialize)]
pub struct TenantToken {
pub token: String,
pub installation_id: u64,
/// Optional narrower scope: only these `owner/name` repositories.
#[serde(default)]
pub repos: Vec<String>,
}

/// Durable store location. See `docs/durable-task-store.md`.
#[derive(Debug, Clone, Deserialize, Serialize)]
pub struct StorageConfig {
Expand Down Expand Up @@ -367,6 +405,52 @@ impl Config {
));
}

// ── Task API auth (issue #3) ────────────────────────────────────
match self.api.mode {
ApiMode::Open => {
if self.api.service_token.is_some() || !self.api.tenants.is_empty() {
out.push(Diagnostic::warning(
"api.mode",
"tokens are configured but api.mode is 'open' — they are ignored; set api.mode = \"token\" to enforce them.",
));
} else {
out.push(Diagnostic::warning(
"api.mode",
"the task API is unauthenticated (api.mode = \"open\") — fine for local development, never expose it publicly; hosted deployments must use \"token\".",
));
}
}
ApiMode::Token => {
if self.api.service_token.is_none() && self.api.tenants.is_empty() {
out.push(Diagnostic::error(
"api.mode",
"api.mode is 'token' but no api.service_token or [[api.tenants]] tokens are configured — every task API call would fail.",
));
}
}
Comment on lines +423 to +430
}
let mut seen_api_tokens = std::collections::HashSet::new();
for candidate in self
.api
.service_token
.iter()
.chain(self.api.tenants.iter().map(|t| &t.token))
{
let trimmed = candidate.trim();
if trimmed.len() < 16 {
out.push(Diagnostic::warning(
"api.tenants[].token",
"an API token is shorter than 16 characters — use a long random string.",
));
}
if !trimmed.is_empty() && !seen_api_tokens.insert(trimmed) {
out.push(Diagnostic::error(
"api.tenants[].token",
"duplicate API token — two scopes would be indistinguishable at the boundary.",
));
}
}
Comment on lines +432 to +452

// ── Review policy ───────────────────────────────────────────────
let known_ids: std::collections::HashSet<&str> =
self.familiars.iter().map(|f| f.id.as_str()).collect();
Expand Down Expand Up @@ -495,6 +579,12 @@ fn next_step_for(field: &str, _message: &str) -> &'static str {
"memory.approval_required" => {
"Keep memory.approval_required = true so learned facts need maintainer review, or accept the risk deliberately."
}
"api.mode" => {
"Set api.mode = \"token\" and configure api.service_token and/or [[api.tenants]] tokens for hosted use."
}
"api.tenants[].token" => {
"Generate long random tokens (e.g. openssl rand -hex 32) and keep each scope's token unique."
}
_ => "Update this config field, then rerun coven-github doctor.",
}
}
Expand Down Expand Up @@ -595,6 +685,7 @@ mod tests {
review: ReviewConfig::default(),
storage: StorageConfig::default(),
memory: MemoryConfig::default(),
api: ApiConfig::default(),
}
}

Expand Down Expand Up @@ -869,6 +960,55 @@ mod tests {
assert!(errs.contains(&"familiars"));
}

#[test]
fn token_mode_without_tokens_is_an_error_and_open_mode_warns() {
let dir = tmpdir();
let pem = write_pem(&dir);
let bin = write_bin(&dir);
let mut cfg = config_with(
GitHubAppConfig {
app_id: 123,
private_key_path: pem,
webhook_secret: "a-long-random-webhook-secret".into(),
api_base_url: None,
},
WorkerConfig {
concurrency: 4,
coven_code_bin: bin,
workspace_root: dir.clone(),
timeout_secs: 600,
max_retries: 2,
},
vec![good_familiar()],
);

// Default open mode: a warning, not an error.
let diags = cfg.check();
assert!(errors(&diags).is_empty(), "diags: {diags:?}");
assert!(
diags
.iter()
.any(|d| d.field == "api.mode" && d.message.contains("unauthenticated")),
"open mode must warn: {diags:?}"
);

// Token mode with nothing to match against would deny every call.
cfg.api.mode = ApiMode::Token;
let diags = cfg.check();
assert!(
errors(&diags).contains(&"api.mode"),
"token mode without tokens must error: {diags:?}"
);

// A configured tenant token clears the error.
cfg.api.tenants = vec![TenantToken {
token: "a-long-random-api-token".into(),
installation_id: 1,
repos: vec![],
}];
assert!(errors(&cfg.check()).is_empty());
}

#[test]
fn first_run_errors_include_operator_next_steps() {
let dir = tmpdir();
Expand Down
98 changes: 93 additions & 5 deletions crates/store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::path::Path;
use std::sync::{Arc, Mutex};

/// Current schema version, stored in `PRAGMA user_version`.
const SCHEMA_VERSION: i32 = 2;
const SCHEMA_VERSION: i32 = 3;

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

/// The Cave oversight projection: every non-reply task, newest first.
/// `familiar_names` maps familiar ids to display names (config-owned).
/// `scope` limits visibility to one tenant (issue #3); `None` is the
/// adapter's own unrestricted view.
pub async fn cave_list(
&self,
familiar_names: HashMap<String, String>,
scope: Option<ApiScope>,
) -> Result<Vec<TaskListItem>> {
let conn = self.conn.clone();
tokio::task::spawn_blocking(move || {
let conn = conn.lock().expect("store mutex poisoned");
let mut stmt = conn.prepare(
"SELECT id, repo, familiar_id, kind, state, result_status,
branch, pr_number, check_run_url, updated_at
branch, pr_number, check_run_url, updated_at,
installation_id
FROM tasks
WHERE json_extract(kind, '$.kind') <> 'command_reply'
AND (?1 IS NULL OR installation_id = ?1)
ORDER BY updated_at DESC, id",
)?;
let rows = stmt.query_map([], |row| {
let installation_filter = scope.as_ref().map(|s| s.installation_id);
let rows = stmt.query_map(params![installation_filter], |row| {
Ok((
row.get::<_, String>(0)?,
row.get::<_, String>(1)?,
Expand All @@ -367,6 +373,7 @@ impl Store {
row.get::<_, Option<u64>>(7)?,
row.get::<_, Option<String>>(8)?,
row.get::<_, String>(9)?,
row.get::<_, u64>(10)?,
))
})?;
let mut items = Vec::new();
Expand All @@ -382,7 +389,16 @@ impl Store {
pr_number,
check_run_url,
updated_at,
_installation_id,
) = row?;
// Repository narrowing within the installation, when scoped.
if let Some(scope) = &scope {
if let Some(repos) = &scope.repos {
if !repos.contains(&repo) {
continue;
}
}
}
let kind: TaskKind = serde_json::from_str(&kind_json)
.context("stored task kind is unreadable")?;
let (issue_number, issue_title) = surface_of(&kind);
Expand Down Expand Up @@ -413,6 +429,62 @@ impl Store {
}
}

/// Tenant visibility limits for task API reads (issue #3).
#[derive(Debug, Clone)]
pub struct ApiScope {
pub installation_id: u64,
/// `None` = every repository in the installation; `Some` narrows further.
pub repos: Option<Vec<String>>,
}

impl Store {
/// Appends a task-API read to the audit trail (issue #3).
pub async fn record_api_read(
&self,
caller: &str,
scope: &str,
action: &str,
result: &str,
) -> Result<()> {
let conn = self.conn.clone();
let (caller, scope, action, result) = (
caller.to_string(),
scope.to_string(),
action.to_string(),
result.to_string(),
);
tokio::task::spawn_blocking(move || {
let conn = conn.lock().expect("store mutex poisoned");
conn.execute(
"INSERT INTO api_audit (at, caller, scope, action, result)
VALUES (?1, ?2, ?3, ?4, ?5)",
params![now_rfc3339(), caller, scope, action, result],
)?;
Ok(())
})
.await
.expect("store task panicked")
}

/// `(caller, scope, action, result)` audit rows, oldest first.
pub async fn api_audit_entries(&self) -> Result<Vec<(String, String, String, String)>> {
let conn = self.conn.clone();
tokio::task::spawn_blocking(move || {
let conn = conn.lock().expect("store mutex poisoned");
let mut stmt =
conn.prepare("SELECT caller, scope, action, result FROM api_audit ORDER BY id")?;
let rows = stmt
.query_map([], |row| {
Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?))
})?
.collect::<std::result::Result<Vec<_>, _>>()?;
Ok(rows)
})
.await
.expect("store task panicked")
}
}

/// Terminal transition applied by [`Store::finish`].
#[derive(Debug, Clone, Default)]
pub struct Terminal {
Expand Down Expand Up @@ -525,6 +597,22 @@ fn migrate(conn: &Connection) -> Result<()> {
conn.execute_batch("ALTER TABLE tasks ADD COLUMN result_status TEXT;")
.context("failed to apply schema v2")?;
}
if version < 3 {
// v3: task API read audit (issue #3).
conn.execute_batch(
r#"
CREATE TABLE api_audit (
id INTEGER PRIMARY KEY AUTOINCREMENT,
at TEXT NOT NULL,
caller TEXT NOT NULL,
scope TEXT NOT NULL,
action TEXT NOT NULL,
result TEXT NOT NULL
);
"#,
)
.context("failed to apply schema v3")?;
}
conn.pragma_update(None, "user_version", SCHEMA_VERSION)?;
Ok(())
}
Expand Down Expand Up @@ -870,7 +958,7 @@ mod queue_tests {
.expect("finish");

let items = store
.cave_list(HashMap::from([("cody".to_string(), "Cody".to_string())]))
.cave_list(HashMap::from([("cody".to_string(), "Cody".to_string())]), None)
.await
.expect("list");
assert_eq!(items.len(), 1);
Expand Down Expand Up @@ -950,7 +1038,7 @@ mod queue_tests {
enqueue(&store, "d1", &fix_task("work")).await;
enqueue(&store, "d2", &reply).await;

let items = store.cave_list(HashMap::new()).await.expect("list");
let items = store.cave_list(HashMap::new(), None).await.expect("list");
assert_eq!(items.len(), 1, "adapter replies are not Cave tasks");
assert_eq!(items[0].id, "work");
assert_eq!(items[0].status, TaskListStatus::Queued);
Expand Down
Loading
Loading