Skip to content

Commit b96014c

Browse files
fix: harden native review publication
1 parent 4a5e5ae commit b96014c

9 files changed

Lines changed: 4646 additions & 317 deletions

File tree

.env.1password.example

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
GITHUB_APP_ID=op://Private/CovenCat/App ID
22
GITHUB_WEBHOOK_SECRET=op://Private/CovenCat/Webhook Secret
33
GITHUB_APP_PRIVATE_KEY=op://Private/CovenCat/Private Key
4+
COVEN_PUBLICATION_SIGNING_SECRET=op://Private/CovenCat/Publication Signing Secret
45
COVEN_GITHUB_POLICY_PATH=./coven-github-policy.json
56
COVEN_GITHUB_STATE_DIR=./coven-github-state
6-
COVEN_CODE_BIN=coven-code
7+
COVEN_RUNTIME_ISOLATION=bwrap
8+
COVEN_RUNTIME_EXTERNAL_ISOLATION=verify-host-controls-before-setting
9+
COVEN_GITHUB_REVOCATION_EVENTS=verify-live-app-before-setting
10+
COVEN_BWRAP_BIN=/usr/bin/bwrap
11+
COVEN_RUNTIME_ROOTFS=/opt/coven-runtime/rootfs
12+
COVEN_RUNTIME_NETWORK=shared
13+
COVEN_CODE_BIN=/usr/local/bin/coven-code

README.md

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ The adapter is deployment-specific. It is not the canonical Rust worker
1010
implementation; it exists so hosted webhook behavior can be reviewed,
1111
reproduced, and changed through PRs instead of server-only edits.
1212

13+
Long-running tasks execute outside the HTTP thread so GitHub deliveries are
14+
acknowledged promptly and health endpoints stay responsive. Queued or
15+
interrupted tasks are discovered again when the service starts. Native review
16+
deployments must subscribe the GitHub App to both `pull_request` and `push`, and
17+
must enable `pull_request.synchronize`, `pull_request.edited`,
18+
`pull_request.reopened`, and the actionless `push` trigger. These deliveries
19+
reconcile stale decisive reviews after head changes, base retargeting, reopening,
20+
or new commits on a PR's base branch. A `publication.mode=comment` route that
21+
lacks any of these safety triggers fails closed instead of publishing native PR
22+
reviews.
23+
1324
## Files
1425

1526
- `src/adapter.ts` - webhook handler, task router, task runner, PR evidence
@@ -32,17 +43,60 @@ The deployment expects secrets and mutable state to be supplied outside git:
3243

3344
- `GITHUB_APP_ID`
3445
- `GITHUB_WEBHOOK_SECRET` or `WEBHOOK_SECRET`
46+
- `COVEN_PUBLICATION_SIGNING_SECRET` - optional dedicated HMAC key for review
47+
identity markers; defaults to the webhook secret for compatibility
48+
- `COVEN_PUBLICATION_PREVIOUS_SIGNING_SECRETS` - comma-separated prior marker
49+
keys retained only during rotation/reconciliation
3550
- `GITHUB_APP_PRIVATE_KEY_PATH` or `.coven-github-private-key.pem`
3651
- `COVEN_GITHUB_STATE_DIR`
3752
- `COVEN_GITHUB_POLICY_PATH`
38-
- `COVEN_CODE_BIN`
53+
- `COVEN_RUNTIME_ISOLATION=bwrap` - required for every non-demo task; unset is
54+
fail-closed and never falls back to direct execution
55+
- `COVEN_RUNTIME_EXTERNAL_ISOLATION=network-egress-and-resource-limits-verified`
56+
- mandatory declaration that the deployment independently enforces egress,
57+
CPU, memory, PID, and disk/scratch limits
58+
- `COVEN_GITHUB_REVOCATION_EVENTS=pull-request-and-push-verified` - set only
59+
after verifying the installed App's live `pull_request` and `push`
60+
subscriptions; native PR publication fails closed without it
61+
- `COVEN_BWRAP_BIN` - absolute host path to bubblewrap (defaults to
62+
`/usr/bin/bwrap`)
63+
- `COVEN_RUNTIME_ROOTFS` - dedicated credential-free runtime rootfs
64+
- `COVEN_CODE_BIN` - absolute coven-code path inside that rootfs
65+
- `COVEN_RUNTIME_NETWORK=shared` - explicit opt-in required when the Codex
66+
provider needs network access; the default is `none`
3967
- `COVEN_REVIEW_FIX_LOOPS` - optional bounded review-fix loop count, clamped
4068
between `0` and `5`; defaults to `0` so hosted repair loops are opt-in
4169
- Codex OAuth tokens under the deployed account's `.coven-code` directory
4270

4371
Do not commit private keys, webhook secrets, OAuth tokens, generated task state,
4472
workspaces, or attempt artifacts.
4573

74+
Real tasks never execute directly as the webhook account. Before minting a
75+
GitHub token, the adapter runs a bubblewrap probe and verifies read-only input,
76+
writable workspace/output mounts, a private PID namespace, no network for
77+
validation, and a dedicated rootfs that does not contain adapter state or
78+
credentials. A missing binary, rootfs, executable, or usable user namespace
79+
records the task as `runtime_isolation_unavailable` with no direct fallback.
80+
`publication.mode=record_only` is not an isolation control.
81+
82+
The runtime rootfs must contain the configured `coven-code`, `git`, and shell
83+
executables plus their libraries, CA/DNS files, and approved runtime assets. It
84+
must not contain the GitHub App key, webhook state, policy, parent home, or Codex
85+
token store. The runtime receives only its dedicated model credential; it never
86+
receives a GitHub token or Git askpass helper. Shared networking is not an
87+
egress-confidentiality boundary, so use a dedicated, revocable model credential
88+
and an externally enforced allowlist that blocks loopback, LAN, and metadata
89+
services. The adapter refuses real tasks unless the external network/resource
90+
isolation declaration is present.
91+
92+
The current runtime passes that model credential to `coven-code`; an untrusted
93+
checkout can therefore try to consume or encode it through the model channel
94+
even when ordinary egress is filtered. Treat this release as trusted-repository
95+
only. Public/untrusted pull requests require a separately constrained worker and
96+
a quota-limited credential broker that never exposes a reusable model token to
97+
the repository process. Do not set the external-isolation declaration for that
98+
use case until those controls exist.
99+
46100
## Local runtime
47101

48102
Install dependencies, build TypeScript, and start the webhook service:
@@ -95,6 +149,16 @@ Deployments should provide `coven-github-policy.json` through
95149
`COVEN_GITHUB_POLICY_PATH`. That file is intentionally ignored because it is
96150
environment-specific.
97151

152+
Every route with `publication.mode=comment` must include all native-review
153+
safety triggers: `pull_request.synchronize`, `pull_request.edited`,
154+
`pull_request.reopened`, and `push`. The `push` key is actionless; do not write
155+
`push.`. Both the `pull_request` and `push` webhook events must also be enabled
156+
on the installed GitHub App. Missing policy coverage is a configuration error
157+
and native publication remains fail closed. After verifying the live App
158+
registration, set
159+
`COVEN_GITHUB_REVOCATION_EVENTS=pull-request-and-push-verified`; this declaration
160+
is required in addition to the policy trigger list.
161+
98162
Start from [`config/example-policy.json`](config/example-policy.json) and the
99163
connection guide in
100164
[`docs/coven-github-connection.md`](docs/coven-github-connection.md).
@@ -114,8 +178,17 @@ connection guide in
114178
remain in the review body. Decisive reviews are bound to the captured commit
115179
and require full changed-file coverage, a clean matching checkout, and
116180
verified passing test evidence. They are created pending, then submitted only
117-
after a fresh head check; a concurrent head change causes a COMMENT downgrade
118-
or automatic dismissal.
181+
after fresh head and base checks; a concurrent revision change causes a
182+
COMMENT downgrade or automatic dismissal. Passing claims are decisive only
183+
when they match successful host-captured validation receipts. Validation and
184+
post-run Git checks execute in a second credential-free, network-disabled
185+
sandbox.
186+
- Uses repository-scoped installation tokens: parent Git gets only
187+
`contents:read`, PR evidence gets read authority, and publication write
188+
authority is minted only after isolated execution has finished.
189+
- Persists `publication_pending` before GitHub writes and resumes interrupted
190+
publication on startup or duplicate webhook delivery without rerunning the
191+
agent.
119192
- Persists publication identities and review/comment IDs in the configured
120193
state directory, reconciles HMAC-signed App-authored identities with GitHub,
121194
and serializes publication per PR so retries and concurrent runs do not

config/example-policy.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
"enabled_triggers": [
88
"issues.labeled",
99
"issue_comment.created",
10-
"pull_request_review_comment.created"
10+
"pull_request_review_comment.created",
11+
"pull_request.synchronize",
12+
"pull_request.edited",
13+
"pull_request.reopened",
14+
"push"
1115
],
1216
"trigger_labels": [
1317
"coven:fix",

docs/coven-github-connection.md

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,40 @@ Set the manifest webhook URL to this service:
2121
https://your-host/webhook
2222
```
2323

24-
The manifest subscribes to the events this adapter can route:
24+
The adapter can route these GitHub App webhook events:
2525

2626
- `issues`
2727
- `issue_comment`
28+
- `pull_request`
2829
- `pull_request_review`
2930
- `pull_request_review_comment`
3031
- `check_suite`
3132
- `check_run`
33+
- `push`
34+
35+
For every route using `publication.mode: comment`, the installed GitHub App must
36+
subscribe to both **Pull request** (`pull_request`) and **Push** (`push`) events.
37+
The route's `enabled_triggers` must contain all four safety triggers:
38+
39+
- `pull_request.synchronize` for new commits or force-pushes on the PR head.
40+
- `pull_request.edited` for base-branch retargeting.
41+
- `pull_request.reopened` to reconcile a PR when it becomes active again.
42+
- `push` to reconcile open PRs when new commits land on their base branch. Push
43+
deliveries have no `action`, so the policy key is exactly `push`, not `push.`.
44+
45+
These deliveries let the adapter dismiss signed, decisive covencat reviews
46+
whose reviewed head/base pair is no longer current. Native publication fails
47+
closed if any required policy trigger is absent. `doctor:app` reports each
48+
missing trigger as an error. Keep the route on `record_only` until both the
49+
policy and App subscriptions are complete. After checking the live App settings,
50+
set `COVEN_GITHUB_REVOCATION_EVENTS=pull-request-and-push-verified`; this explicit
51+
deployment declaration is also required for native PR publication.
52+
53+
Treat the manifest file and the live App registration as separate checks:
54+
ensure `docs/app-manifest.json` in `OpenCoven/coven-github` lists both events,
55+
then verify the installed App's settings also subscribe to **Pull request** and
56+
**Push**. Updating a manifest file does not retroactively update an existing
57+
GitHub App registration.
3258

3359
After creating the App, keep these values outside git:
3460

@@ -68,6 +94,11 @@ Then replace:
6894
- `bot_usernames` with the GitHub App bot login, for example
6995
`coven-cody[bot]`.
7096
- `trigger_labels` with labels that should start tasks.
97+
- `enabled_triggers` with the exact event/action pairs allowed to spend compute,
98+
plus actionless `push` where needed. Events not listed are acknowledged and
99+
ignored. A `publication.mode: comment` route must include all four
100+
native-review safety triggers listed above; otherwise publication fails
101+
closed.
71102
- `familiar` with the familiar id, display name, model, and skills to pass to
72103
`coven-code`.
73104

@@ -79,7 +110,14 @@ export COVEN_GITHUB_POLICY_PATH="$PWD/coven-github-policy.json"
79110

80111
Keep `publication.mode` as `record_only` for first smoke runs. Switch it to
81112
`comment` only after you have verified the App installation, `coven-code`
82-
runtime, Codex token, and workspace permissions.
113+
runtime, Codex token, workspace permissions, both required GitHub App webhook
114+
subscriptions, and all four required safety triggers.
115+
116+
`record_only` controls publication, not process isolation. Non-demo work stays
117+
blocked until the mandatory runtime sandbox below passes its executable probe.
118+
For decisive native reviews, also configure a bounded list of trusted
119+
validation commands under `publication.validation_commands`; a runtime-authored
120+
claim without a matching successful sandbox receipt is published as COMMENT.
83121

84122
## Runtime Checklist
85123

@@ -92,12 +130,51 @@ export GITHUB_WEBHOOK_SECRET="replace-with-github-secret"
92130
export GITHUB_APP_PRIVATE_KEY_PATH="$PWD/keys/coven-github.private-key.pem"
93131
export COVEN_GITHUB_POLICY_PATH="$PWD/coven-github-policy.json"
94132
export COVEN_GITHUB_STATE_DIR="$PWD/coven-github-state"
95-
export COVEN_CODE_BIN="$(command -v coven-code)"
133+
install -d -m 700 "$COVEN_GITHUB_STATE_DIR"
134+
export COVEN_RUNTIME_ISOLATION="bwrap"
135+
export COVEN_RUNTIME_EXTERNAL_ISOLATION="network-egress-and-resource-limits-verified"
136+
export COVEN_GITHUB_REVOCATION_EVENTS="pull-request-and-push-verified"
137+
export COVEN_BWRAP_BIN="/usr/bin/bwrap"
138+
export COVEN_RUNTIME_ROOTFS="/opt/coven-runtime/rootfs"
139+
export COVEN_CODE_BIN="/usr/local/bin/coven-code" # path inside the rootfs
140+
export COVEN_RUNTIME_NETWORK="shared" # explicit Codex egress opt-in
96141

97142
npm run doctor:app
98143
npm start
99144
```
100145

146+
Build the dedicated rootfs as an administrator-controlled deployment artifact.
147+
It must contain the configured `coven-code`, `/usr/bin/git`, `/bin/sh`,
148+
`/bin/true`, their libraries, CA/DNS files, and required runtime assets. Do not
149+
copy the webhook checkout, state directory, policy, App key, hosting-user home,
150+
or `.coven-code` token store into it. Bubblewrap must be able to create user
151+
namespaces on the target host; `doctor:app` runs a real read/write isolation
152+
probe and fails when it cannot.
153+
154+
The external-isolation declaration is intentionally mandatory. Set it only
155+
after the worker host or container enforces allowlisted egress (including no
156+
loopback, LAN, or metadata access) and CPU, memory, PID, workspace/disk, and
157+
scratch limits. Bubblewrap mount namespaces and timeouts do not provide those
158+
controls. A state directory from an older deployment must be owned by the
159+
service user and made inaccessible to group/other (for example, `chmod -R go-rwx
160+
"$COVEN_GITHUB_STATE_DIR"`) before this version starts.
161+
162+
This release passes the model credential to `coven-code`, so an untrusted
163+
checkout can still try to consume or encode it through the allowed model
164+
channel. Limit real execution to trusted repositories. Supporting public or
165+
otherwise untrusted pull requests requires a separately constrained worker and
166+
a quota-limited credential broker that does not expose a reusable model token
167+
to repository code; the declaration above must remain unset until that boundary
168+
is actually deployed.
169+
170+
The adapter mounts only per-task input (read-only), the checkout and result
171+
directory (writable), and the checkout `.git` directory again as read-only. It
172+
passes no GitHub token, askpass helper, App secret, SSH agent, or parent home to
173+
`coven-code`. Publication authority is minted only after the sandbox exits.
174+
Validation commands run again without credentials or network access. If the
175+
host cannot satisfy this boundary, leave real execution disabled and use demo
176+
mode or an externally isolated worker; there is no unsafe direct fallback.
177+
101178
In another shell:
102179

103180
```bash

scripts/doctor-app-config.mjs

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
11
#!/usr/bin/env node
2-
import {existsSync, readFileSync} from "node:fs";
2+
import {existsSync, mkdtempSync, readFileSync, rmSync} from "node:fs";
3+
import {tmpdir} from "node:os";
34
import {join} from "node:path";
45

5-
import {createConfig} from "../dist/src/adapter.js";
6+
import {createConfig, probeRuntimeIsolation, runtimeIsolationIssue} from "../dist/src/adapter.js";
7+
8+
const NATIVE_REVIEW_SAFETY_TRIGGERS = [
9+
"pull_request.synchronize",
10+
"pull_request.edited",
11+
"pull_request.reopened",
12+
"push",
13+
];
614

715
function finding(level, field, message, next) {
816
return {level, field, message, next};
@@ -45,6 +53,7 @@ if (!config.privateKeyPem && !existsSync(config.privateKeyPath)) {
4553
}
4654

4755
const policy = loadJson(config.policyPath);
56+
let requiresRevocationEvents = false;
4857
if (!policy) {
4958
findings.push(finding("error", "COVEN_GITHUB_POLICY_PATH", "policy file is missing or invalid JSON", "Copy config/example-policy.json and replace the installation/repository IDs."));
5059
} else {
@@ -64,11 +73,49 @@ if (!policy) {
6473
if (Object.keys(repos).includes("987654321")) {
6574
findings.push(finding("error", "policy.repositories", "policy still uses placeholder repository ID 987654321", "Replace it with the real repository ID."));
6675
}
76+
for (const [repositoryId, route] of Object.entries(repos)) {
77+
if (route?.publication?.mode !== "comment") continue;
78+
requiresRevocationEvents = true;
79+
const enabledTriggers = new Set(Array.isArray(route.enabled_triggers) ? route.enabled_triggers.map(String) : []);
80+
for (const trigger of NATIVE_REVIEW_SAFETY_TRIGGERS) {
81+
if (!enabledTriggers.has(trigger)) {
82+
const field = `policy.installations.${installationId}.repositories.${repositoryId}.enabled_triggers`;
83+
findings.push(finding(
84+
"error",
85+
field,
86+
`native review publication requires ${trigger}`,
87+
`Add ${trigger} to enabled_triggers and subscribe the GitHub App to the corresponding webhook event, or use publication.mode=record_only.`,
88+
));
89+
}
90+
}
91+
}
6792
}
6893
}
6994

70-
if (!process.env.COVEN_CODE_BIN && !config.demoMode) {
71-
findings.push(finding("warning", "COVEN_CODE_BIN", "COVEN_CODE_BIN is not set", "Set it to the coven-code binary path before running real tasks."));
95+
if (requiresRevocationEvents && !config.revocationEventsVerified) {
96+
findings.push(finding(
97+
"error",
98+
"COVEN_GITHUB_REVOCATION_EVENTS",
99+
"native review publication requires verified pull_request and push delivery subscriptions on the installed GitHub App",
100+
"Verify the live App subscriptions, then set COVEN_GITHUB_REVOCATION_EVENTS=pull-request-and-push-verified; otherwise use publication.mode=record_only.",
101+
));
102+
}
103+
104+
if (!config.demoMode) {
105+
const isolationIssue = runtimeIsolationIssue(config);
106+
if (isolationIssue) {
107+
findings.push(finding("error", "COVEN_RUNTIME_ISOLATION", isolationIssue, "Configure COVEN_RUNTIME_ISOLATION=bwrap with a dedicated rootfs and absolute runtime binary paths. There is no direct-execution fallback."));
108+
} else {
109+
const probeDir = mkdtempSync(join(tmpdir(), "coven-runtime-doctor-"));
110+
try {
111+
const probeIssue = probeRuntimeIsolation(config, probeDir);
112+
if (probeIssue) {
113+
findings.push(finding("error", "COVEN_RUNTIME_ISOLATION", probeIssue, "Fix bubblewrap/user-namespace support or keep real task execution disabled."));
114+
}
115+
} finally {
116+
rmSync(probeDir, {recursive: true, force: true});
117+
}
118+
}
72119
}
73120

74121
const errors = findings.filter((item) => item.level === "error");
@@ -81,6 +128,10 @@ const output = {
81128
private_key: Boolean(config.privateKeyPem || existsSync(config.privateKeyPath)),
82129
policy_path: config.policyPath,
83130
state_dir: config.stateDir,
131+
runtime_isolation: config.runtimeIsolation,
132+
runtime_external_isolation: config.runtimeExternalIsolationVerified,
133+
revocation_events: config.revocationEventsVerified,
134+
runtime_rootfs: config.runtimeRootfs || null,
84135
},
85136
findings,
86137
};

0 commit comments

Comments
 (0)