ci: preauthorize skill review waiver hashes (#5143)

This commit is contained in:
Willem Jiang 2026-09-02 16:54:23 +08:00 committed by GitHub
parent 30788c79ff
commit eac028cca6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 138 additions and 15 deletions

View File

@ -9,6 +9,9 @@
"line": 35,
"evidence": "subprocess.run",
"file_sha256": "sha256:87d864570220b699fac52da309d2d6efdb060647bfebc74f768128e646accf80",
"preapproved_file_sha256s": [
"sha256:2877bde08bf3f437b9dae3d57585a0840b9b1024736d2e5c6c657b71899269d0"
],
"reason": "Required Claude CLI invocation uses a fixed executable, an argv list, prompt input over stdin, and shell=False.",
"expires_on": "2027-02-28"
},
@ -20,6 +23,9 @@
"line": 85,
"evidence": "subprocess.Popen",
"file_sha256": "sha256:43e3b8f80dbf69c343967ba77e268fae991d9fa3ed68b32a0ff02532cd48657f",
"preapproved_file_sha256s": [
"sha256:ea2521ba41c8fd16b2900758c890bb6c6d2b4b01da10b4a860facb8587ed0bde"
],
"reason": "Required Claude CLI invocation uses a fixed executable, an argv list, captured output streams, and shell=False.",
"expires_on": "2027-02-28"
}

View File

@ -105,9 +105,13 @@ Skill quality review note:
edits from their head revision, but only the manifest from the trusted base
revision can suppress that run. Entries match one error finding exactly,
include the reviewed file's SHA-256 and an expiry date, remain visible in CI
output, and can never waive blocker findings. Adding a waiver and relying on
it therefore requires two steps: merge the reviewed waiver first, then update
the affected public skill in a later pull request.
output, and can never waive blocker findings. An entry may also preapprove a
bounded list of future full-file SHA-256 values. Those hashes become effective
only after the manifest change lands in the trusted base. Adding or changing a
waiver and relying on it therefore requires two steps: merge the reviewed
manifest change first, then update the affected public skill in a later pull
request. After that skill change lands, promote its hash to `file_sha256` and
remove the consumed preapproval in a follow-up manifest cleanup.
Scheduled-task note:
- The scheduled-task MVP adds a workspace page at `/workspace/scheduled-tasks` plus a background scheduler service gated by `config.yaml -> scheduler.enabled`.

View File

@ -976,6 +976,8 @@ cd backend
uv run python -m deerflow.skills.review.cli ../skills/public/data-analysis --format text --fail-on error --fail-on-incomplete
```
Public-skill CI waivers are exact, expiring exceptions in `.github/skill-review-waivers.v1.json`. Because only the trusted base manifest can suppress a finding, a file-changing pull request can be preauthorized safely by first merging a manifest-only change that lists the reviewed future full-file SHA-256 in `preapproved_file_sha256s`; the file change can then land in a later pull request.
Tools follow the same philosophy. DeerFlow comes with a core toolset — web search, web fetch, rendered web capture, file operations, bash execution — and supports custom tools via MCP servers and Python functions. The bundled DDG, Brave, Tavily, and SearXNG search providers accept an optional `time_range` of `day`, `week`, `month`, or `year`; omitting it preserves existing search behavior. For DDG recency searches, DeerFlow excludes DDGS backends that ignore time limits. Swap anything. Add anything.
Advanced deployments can enable pluggable authorization with `authorization.enabled` in `config.yaml`. A configured `AuthorizationProvider` filters denied tools before they reach the model or deferred-tool catalog, then the same provider is checked again before every business-tool execution through the existing guardrail middleware. Gateway `threads:*` and `runs:*` route permissions are derived from the same provider, while existing owner checks and admin-only management gates remain in force. Every HTTP route that starts or enables a future Agent run requires `runs:create`: this includes the stateless `POST /api/runs/stream` and `POST /api/runs/wait` endpoints plus scheduled-task create, update, resume, and manual-trigger mutations. Scheduled-task mutations retain their existing `threads:write` requirement, and the stateless routes separately enforce ownership when the optional thread ID is supplied in the request body. A generated `tool_search` may bypass the second tool check only when it fronts the current build's already-filtered deferred catalog. Model access follows the same provider: the Gateway `models` list is filtered per principal, `model:use` is enforced on model detail requests and again when the runtime resolves the agent's model, and a denied default model falls back to the first remaining candidate that also passes `model:use`. The built-in RBAC provider supports per-role `tools`, `routes`, `models`, `skills`, and `sandbox` allow/deny policies and validates that `default_role` names a configured role; authorization is disabled by default. See `config.example.yaml` and the [authorization RFC](docs/plans/2026-07-10-pluggable-authorization-rfc.md).

View File

@ -34,7 +34,12 @@ def _write_target(repo_root: Path, content: bytes = b"safe subprocess invocation
return target, f"sha256:{hashlib.sha256(content).hexdigest()}"
def _waiver(*, digest: str, expires_on: date = date(2027, 2, 28)) -> SkillReviewWaiver:
def _waiver(
*,
digest: str,
expires_on: date = date(2027, 2, 28),
preapproved_file_sha256s: tuple[str, ...] = (),
) -> SkillReviewWaiver:
return SkillReviewWaiver(
package="skills/public/demo",
source="skillscan",
@ -45,6 +50,7 @@ def _waiver(*, digest: str, expires_on: date = date(2027, 2, 28)) -> SkillReview
file_sha256=digest,
reason="Fixed executable and argv invocation with shell disabled.",
expires_on=expires_on,
preapproved_file_sha256s=preapproved_file_sha256s,
)
@ -60,7 +66,13 @@ def _finding(*, severity: str = "error", line: int = 12) -> dict[str, object]:
}
def _payload(*, digest: str, path: str = "scripts/run.py", duplicate: bool = False) -> bytes:
def _payload(
*,
digest: str,
path: str = "scripts/run.py",
duplicate: bool = False,
preapproved_file_sha256s: object | None = None,
) -> bytes:
entry = {
"package": "skills/public/demo",
"source": "skillscan",
@ -72,6 +84,8 @@ def _payload(*, digest: str, path: str = "scripts/run.py", duplicate: bool = Fal
"reason": "Fixed executable and argv invocation with shell disabled.",
"expires_on": "2027-02-28",
}
if preapproved_file_sha256s is not None:
entry["preapproved_file_sha256s"] = preapproved_file_sha256s
return json.dumps({"schema_version": SCHEMA_VERSION, "waivers": [entry, entry] if duplicate else [entry]}).encode()
@ -84,6 +98,8 @@ def test_committed_manifest_matches_schema_and_strict_parser() -> None:
parsed = parse_manifest(manifest_path.read_bytes(), source=str(manifest_path))
assert len(parsed.waivers) == 2
assert parsed.waivers[0].preapproved_file_sha256s == ("sha256:2877bde08bf3f437b9dae3d57585a0840b9b1024736d2e5c6c657b71899269d0",)
assert parsed.waivers[1].preapproved_file_sha256s == ("sha256:ea2521ba41c8fd16b2900758c890bb6c6d2b4b01da10b4a860facb8587ed0bde",)
def test_skill_creator_waivers_match_current_error_findings() -> None:
@ -115,6 +131,27 @@ def test_parser_rejects_duplicate_exact_waivers(tmp_path: Path) -> None:
parse_manifest(_payload(digest=digest, duplicate=True), source="test manifest")
@pytest.mark.parametrize(
("preapproved", "error"),
[
("sha256:" + "1" * 64, "must be an array"),
(["sha256:" + "1" * 63], "64 lowercase hex characters"),
(["sha256:" + "1" * 64] * 2, "entries must be unique"),
(["PRIMARY"], "must not repeat file_sha256"),
([f"sha256:{index:064x}" for index in range(9)], "at most 8 entries"),
],
)
def test_parser_rejects_invalid_preapproved_hashes(tmp_path: Path, preapproved: object, error: str) -> None:
_, digest = _write_target(tmp_path)
value = [digest] if preapproved == ["PRIMARY"] else preapproved
with pytest.raises(WaiverManifestError, match=error):
parse_manifest(
_payload(digest=digest, preapproved_file_sha256s=value),
source="test manifest",
)
def test_missing_manifest_at_ref_means_no_waivers(tmp_path: Path, monkeypatch) -> None:
monkeypatch.setattr(
waiver_support.subprocess,
@ -138,6 +175,47 @@ def test_matching_waiver_requires_exact_finding_and_current_file_hash(tmp_path:
assert matching_waiver(_finding(), package="skills/public/demo", manifest=manifest, repo_root=tmp_path, today=date(2026, 8, 31)) is None
def test_preapproved_file_hash_authorizes_a_later_file_revision(tmp_path: Path) -> None:
target, current_digest = _write_target(tmp_path)
future_content = b"safe subprocess invocation with explicit UTF-8\n"
future_digest = f"sha256:{hashlib.sha256(future_content).hexdigest()}"
waiver = _waiver(digest=current_digest, preapproved_file_sha256s=(future_digest,))
manifest = WaiverManifest((waiver,))
facts = {"findings": [_finding()]}
assert (
validate_manifest_against_facts(
manifest,
facts_by_package={waiver.package: facts},
repo_root=tmp_path,
today=date(2026, 8, 31),
)
== []
)
target.write_bytes(future_content)
assert (
matching_waiver(
_finding(),
package=waiver.package,
manifest=manifest,
repo_root=tmp_path,
today=date(2026, 8, 31),
)
is waiver
)
assert (
validate_manifest_against_facts(
manifest,
facts_by_package={waiver.package: facts},
repo_root=tmp_path,
today=date(2026, 8, 31),
)
== []
)
def test_matching_waiver_rejects_symlinked_package_outside_repository(tmp_path: Path) -> None:
external_root = tmp_path.parent / f"{tmp_path.name}-external"
external_target, digest = _write_target(external_root)

View File

@ -20,6 +20,12 @@
"line": { "type": "integer", "minimum": 1 },
"evidence": { "type": "string", "minLength": 1 },
"file_sha256": { "type": "string", "pattern": "^sha256:[0-9a-f]{64}$" },
"preapproved_file_sha256s": {
"type": "array",
"maxItems": 8,
"uniqueItems": true,
"items": { "type": "string", "pattern": "^sha256:[0-9a-f]{64}$" }
},
"reason": { "type": "string", "minLength": 20 },
"expires_on": { "type": "string", "format": "date" },
"approved_in": { "type": "string", "minLength": 1 }

View File

@ -22,15 +22,19 @@ CI-only exceptions from `.github/skill-review-waivers.v1.json`. The manifest is
versioned by `contracts/skill_review/waiver_manifest.v1.schema.json`; each entry
must identify one current error by package, source, rule, path, line, and
evidence, and pin the complete source file with SHA-256 plus an expiry date.
Blockers are never waivable, and waived errors are still printed with their
original severity and justification.
An optional, bounded `preapproved_file_sha256s` list authorizes reviewed future
full-file digests without relaxing the exact finding match. Blockers are never
waivable, and waived errors are still printed with their original severity and
justification.
For pull requests, only the base revision's manifest is effective. The head
manifest is parsed and checked against the current analyzer output, but cannot
self-authorize a finding in the same pull request. Push comparisons use the
same before/after trust boundary. A waiver-only change can therefore land
without weakening its own check, then become effective for later changes after
it is part of the trusted base.
it is part of the trusted base. Preapproved digests must be code-reviewed in
that first change; after the corresponding file revision lands, promote the
consumed digest to `file_sha256` and remove it from the preapproval list.
## Backend Static Analysis Commands

View File

@ -26,7 +26,8 @@ _REQUIRED_ENTRY_FIELDS = {
"reason",
"expires_on",
}
_OPTIONAL_ENTRY_FIELDS = {"approved_in"}
_OPTIONAL_ENTRY_FIELDS = {"approved_in", "preapproved_file_sha256s"}
_MAX_PREAPPROVED_FILE_SHA256S = 8
class WaiverManifestError(ValueError):
@ -45,11 +46,16 @@ class SkillReviewWaiver:
reason: str
expires_on: date
approved_in: str | None = None
preapproved_file_sha256s: tuple[str, ...] = ()
@property
def finding_key(self) -> tuple[str, str, str, int, str]:
return (self.source, self.rule_id, self.path, self.line, self.evidence)
@property
def approved_file_sha256s(self) -> tuple[str, ...]:
return (self.file_sha256, *self.preapproved_file_sha256s)
@dataclass(frozen=True)
class WaiverManifest:
@ -100,9 +106,17 @@ def parse_manifest(payload: bytes | str, *, source: str) -> WaiverManifest:
source_name = _nonempty_string(raw["source"], field=f"{entry_source}.source")
rule_id = _nonempty_string(raw["rule_id"], field=f"{entry_source}.rule_id")
evidence = _nonempty_string(raw["evidence"], field=f"{entry_source}.evidence")
file_sha256 = _nonempty_string(raw["file_sha256"], field=f"{entry_source}.file_sha256")
if not _SHA256_RE.fullmatch(file_sha256):
raise WaiverManifestError(f"{entry_source}.file_sha256: must be sha256 followed by 64 lowercase hex characters")
file_sha256 = _parse_sha256(raw["file_sha256"], field=f"{entry_source}.file_sha256")
raw_preapproved_hashes = raw.get("preapproved_file_sha256s", [])
if not isinstance(raw_preapproved_hashes, list):
raise WaiverManifestError(f"{entry_source}.preapproved_file_sha256s: must be an array")
if len(raw_preapproved_hashes) > _MAX_PREAPPROVED_FILE_SHA256S:
raise WaiverManifestError(f"{entry_source}.preapproved_file_sha256s: must contain at most {_MAX_PREAPPROVED_FILE_SHA256S} entries")
preapproved_file_sha256s = tuple(_parse_sha256(value, field=f"{entry_source}.preapproved_file_sha256s[{hash_index}]") for hash_index, value in enumerate(raw_preapproved_hashes))
if len(set(preapproved_file_sha256s)) != len(preapproved_file_sha256s):
raise WaiverManifestError(f"{entry_source}.preapproved_file_sha256s: entries must be unique")
if file_sha256 in preapproved_file_sha256s:
raise WaiverManifestError(f"{entry_source}.preapproved_file_sha256s: must not repeat file_sha256")
reason = _nonempty_string(raw["reason"], field=f"{entry_source}.reason")
if len(reason) < 20:
raise WaiverManifestError(f"{entry_source}.reason: must contain at least 20 characters")
@ -125,6 +139,7 @@ def parse_manifest(payload: bytes | str, *, source: str) -> WaiverManifest:
reason=reason,
expires_on=expires_on,
approved_in=approved_in,
preapproved_file_sha256s=preapproved_file_sha256s,
)
identity = (*waiver.finding_key, waiver.package)
if identity in identities:
@ -204,7 +219,7 @@ def matching_waiver(
continue
if waiver.expires_on < current_date:
continue
if file_sha256(repo_root, waiver) != waiver.file_sha256:
if file_sha256(repo_root, waiver) not in waiver.approved_file_sha256s:
continue
return waiver
return None
@ -240,8 +255,9 @@ def validate_manifest_against_facts(
errors.append(f"{description}: waivers may target error findings only; blockers can never be waived")
continue
actual_hash = file_sha256(repo_root, waiver)
if actual_hash != waiver.file_sha256:
errors.append(f"{description}: file digest changed (expected {waiver.file_sha256}, found {actual_hash or 'unavailable'})")
if actual_hash not in waiver.approved_file_sha256s:
expected_hashes = ", ".join(waiver.approved_file_sha256s)
errors.append(f"{description}: file digest changed (expected one of {expected_hashes}, found {actual_hash or 'unavailable'})")
return errors
@ -261,6 +277,13 @@ def _nonempty_string(value: object, *, field: str) -> str:
return value
def _parse_sha256(value: object, *, field: str) -> str:
digest = _nonempty_string(value, field=field)
if not _SHA256_RE.fullmatch(digest):
raise WaiverManifestError(f"{field}: must be sha256 followed by 64 lowercase hex characters")
return digest
def _parse_date(value: object, *, field: str) -> date:
text = _nonempty_string(value, field=field)
try: