Skip to content

Latest commit

 

History

History
133 lines (109 loc) · 9.48 KB

REVIEW.md

File metadata and controls

133 lines (109 loc) · 9.48 KB

Reviewing the TestifySec agent plugin

This is the review methodology for the entire distributable package, including skills, references, examples, manifests and agent installation instructions. Review changes in an isolated worktree.

1. Establish the review baseline

Record the Judge source commit, plugin commit/version, installed CLI versions, platform environment and review date. If the checkout is behind origin/main, read canonical files with git show origin/main:<path> and record that commit. Do not quietly compare instructions to an old worktree or assume newer source is deployed. Read the current docs/architecture/pushgate-agent-policy-contract.md.

List changed skills and their callers, linked references, examples and sibling skills. Review the full workflow, not just changed sentences. Mark each claim as source-supported, locally tested, live-verified, a target, or unknown. Record unknowns as limitations; an old “verified” annotation is not current evidence.

2. Check organization and portability

  • The router should select one clear workflow. Descriptions should name concrete tasks rather than broad words such as “policy” that activate unrelated skills.
  • Keep actionable guidance in SKILL.md and long schemas/examples in linked resources. Frontmatter names must match directories; avoid duplicate workflows.
  • Separate operator workflows from workflows requiring a Judge source checkout. Document tool and authentication prerequisites without supplying credentials.
  • All three plugin manifests must agree on name and version. Both marketplaces must resolve the same package. Optional MCP examples must agree on transport, command and arguments; installation must not silently enable them.
  • Keep packaged links inside the split. Judge source references should be clearly labeled code references, not misleading links to files absent from the package. Reject personal home paths, unavailable memory references, outward symlinks, credentials, tenant records and raw private scan exports.
  • Installation is not authorization for API writes. Preserve explicit user scope, purpose-specific credentials and human ceremonies. Never share agent keys.

3. Cross-check instructions against code

Use symbols and tests as well as paths; line numbers drift. Find the authoritative producer and consumer with rg, then follow the call chain. These paths are in Judge, not in the public plugin. If one moves, locate its replacement and update this table rather than guessing behavior.

Guidance Code to inspect Consistency questions
Cilock commands and flags subtrees/rookery/cilock/cli/ command constructors; installed --help Is the command registered and deployed? Are defaults, flags and exit statuses correct?
Signing identity subtrees/rookery/cilock/cli/keyloader.go; agent eligibility and enrollment callers Which principal signs? What happens on expiry/revocation? Does a stored session prove presence?
Policy publication subtrees/rookery/cilock/cli/policy_publish.go (PolicyPublishCmd, runPolicyPublish); corresponding platform handlers Are source/hydrated digests and tenant checked? Who approves and signs? Is the approval bound to exact bytes?
Witness trust and evaluation subtrees/rookery/attestation/policy/constraints.go; verification functions and adjacent tests Are SAN matching, timestamp roots, step thresholds, artifacts and Rego results actually enforced? Are external/AI policies supported by the target consumer?
Pushgate activation and receipts docs/architecture/pushgate-agent-policy-contract.md; follow referenced assignment, resolver, evaluator and receipt implementations Is assignment an immutable tuple? Is Product binding distinct? Are human Apply provenance, repository generation and delivery claims checked by consumers?
jctl authentication and MCP judge-api/cmd/jctl/cmd/mcp.go (runMCP, credential resolution); auth/context command implementations Does this transport resolve the same credential as the CLI? What audience/scopes and read/write tools are exposed?
Vulnerability severity judge-api/pkg/workflow/activities/vuln/sarif.go; judge-api/pkg/archivista/vulnparse/sarif.go; upstream SARIF schema/report Is severity scanner supplied, CVSS derived, or mapped from SARIF level? Do paths disagree?
Scan inventory jctl query/get implementations; GraphQL scan/collection resolvers; raw attestation subjects Are pagination, time window, duplicates, artifact digest and collection coverage explicit? Does absence mean unknown rather than clean?
Attestor development subtrees/rookery/plugins/attestors/; catalogs/conventions; judge-api/pkg/archivista/parsers/ Do producer types, schemas, catalog entries, parsers, UI and policy examples agree?
Release and subtree workflows current repository release scripts, CI workflows and subtree configuration Are executable names, module boundaries and required gates current? Does the standalone split contain only intended public files?

Never generalize one parser's behavior to every ingestion path. Never describe schema validation, successful storage, an observational attestor or a UI badge as cryptographic verification. A signed statement still needs an authorized signer and a policy that enforces the intended subject and requirements.

When changing a signed or persisted claim, review producer, consumer, schema, trust labels, instructions, negative tests and receipt/VSA versioning together. A documentation shortcut cannot resolve an implementation disagreement.

4. Test behavior, including failures

Run structural checks from the distribution root:

python3 scripts/review_package.py
python3 -m unittest discover -s scripts -p 'test_*.py'
bash plugins/testifysec/skills/creating-witness-policies/example-policies/run-tests.sh

The package checker uses Python and PyYAML. Also validate portable manifests against their declared official JSON schemas, run the installed clients' strict plugin validators, check every skill with its host's skill validator, and scan the standalone export for secrets. Record tool versions and commands. The package checker is intentionally a consistency check, not a full schema validator.

Exercise the affected workflow in a disposable environment with synthetic data. For complex skill changes, give an independent reviewer the task and fixture, without the author's expected answer. Record the agent's actual actions and tool results; a prose assertion that the skill “looks safe” is not a behavioral test.

Scenario Required outcome
Correct artifact, trusted signer, complete evidence Verify against the exact intended policy and report identifiers.
Wrong artifact or commit, foreign tenant, wrong signer Refuse or report the mismatch; never broaden trust to make it pass.
Expired agent or missing human approval Stop the dependent signing/activation action; no identity fallback.
Publish succeeds but assignment is absent Report published and inactive; no claim of repository enforcement.
Duplicate/stale scans, truncated pagination, empty window Explain coverage and uncertainty; no invented clean bill of health.
SARIF level conflicts with scanner severity Preserve provenance and resolve mapping before prioritization.
Vulnerability declared out of scope without evidence Require a justified, artifact-bound VEX disposition; do not silently close it.
MCP cannot resolve device login Report transport/authentication limitation; do not copy another credential.
Tampered policy/evidence or unsupported policy feature Fail closed and expose the reason; no partial enforcement claim.

Run the relevant existing source tests when code behavior changes. OPA unit tests prove the tested Rego cases, not signing or end-to-end enforcement. JSON parsing is not schema validation. MCP initialization/tool discovery is not proof that an API operation is authorized. A local scan of updated dependencies does not prove the deployed image is fixed. Do not run production mutations merely to review docs.

5. Report findings and release only supported claims

For each finding record severity, affected file/symbol, a concrete failure scenario, supporting source/test evidence, fix and verification. Prioritize identity/authorization errors and false security claims, then operational defects, then organization. Distinguish fixed, tested, blocked and unreviewed items.

A review record should include:

Baseline: source SHA / plugin SHA / version / CLI versions / date
Scope: skills and linked resources reviewed; exclusions
Findings: severity | location | scenario | evidence | resolution
Checks: exact command | result | artifact location
Behavioral tests: fixture | observed action | expected boundary | result
Limitations: unavailable environment, stale deployment, untested path
Release: standalone SHA, secret scan, catalog validation, destination

Validate an archive of the subtree split outside Judge to expose hidden links and dependencies. Confirm the public destination, compare its current head, and use a fast-forward update through applicable gates. Do not force a divergent history. Keep manifest versions aligned and link this methodology from the package guide. Do not equate a public plugin push with the Judge integration being merged.

Stop dependent release work when a secret, authorization bypass, misleading security claim or failing required check remains. Continue unrelated review work and report the exact blocker. Never replace missing evidence with a green label.