feat: add managed-validation mode (made validate --managed) - #22
Draft
douglasjarquin wants to merge 26 commits into
Draft
feat: add managed-validation mode (made validate --managed)#22douglasjarquin wants to merge 26 commits into
douglasjarquin wants to merge 26 commits into
Conversation
Add Made managed-validation mode: a short-lived, daemonless execution shape that Consigliere invokes to validate an immutable input commit SHA. ## What this adds ### New command made validate --managed --json-events [flags] Validates a workspace against an exact input SHA, emitting a versioned JSON-Lines event stream to stdout. Never waits for human input. Never mutates the workspace. Exits with a typed exit code matching the outcome. ### New packages - internal/managed: contract types, event writer, preflight, decisions, fingerprints, evidence, runner, and lifecycle (run.go) - internal/safegit: hardened Git execution (strips all GIT_* env vars, hooks, fsmonitor, credential helpers); extracted and shared ### Modified packages - internal/pipeline/review: add ReportOnly option (managed mode never applies auto-fix patches or creates commits) - internal/pipeline/document: add RunContextWithBaseSHA for exact-SHA diff ranges (no mutable branch refs in managed mode) - internal/agent/findings: additive Code, Class, Symbol fields (optional, omitempty); schema updated to permit but not require them - internal/config: add ParseBytes for hash-verified config loading - cmd/made/main.go: add validate case - cmd/made/runcommands.go: add validate.managed.v1 to capabilities ### Documents - docs/managed-validation-v1.md: full contract document - docs/managed-validation-integration.md: machine-consumer integration reference ### Golden fixtures - internal/managed/testdata/: passed, needs-decision, failed-retryable, failed-terminal, infrastructure-error, decisions-approved/rejected ## Ownership boundary Made validates. Consigliere orchestrates. Made never waits, pushes, creates PRs, merges, or modifies the workspace. ## Compatibility All existing commands (run submit/status/list/cancel, review decide, daemon, gate, doctor, capabilities) are unchanged and fully compatible. The standalone review auto-fix path is unchanged (ReportOnly defaults false). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MB1: Add base_sha to validation identity - Event envelope now includes base_sha on every event - DecisionsFile adds base_sha binding check - Options gets InvocationID; RunCompletedPayload carries it - StageResult gets JSON struct tags - decisions testdata files include base_sha MB2: Stable finding fingerprints - Fingerprint uses structural identity (code/class/paths/symbol) as primary; description is fallback only, with line/col refs stripped - Two new fingerprint stability tests MB3: Terminal evidence failure forces infrastructure_error - WriteTerminal failure overrides outcome and message before terminal event MB4: Terminal JSON for every post-protocol exit - ValidateOptions performs format-only checks (exit 2, no events) - run.started emitted before all infra work - emitInfraError closure guarantees run.completed on all post-start failures - Evidence dir creation failure produces terminal event MB5: Evidence path containment - run_id hashed via SHA-256 for path-safe directory name - Per-invocation subdirectory prevents evidence overwrite on rerun - EvalSymlinks for canonical workspace path in containment check - Symlink-swap-safe config read (Lstat + Open + Fstat verify) - Unique tmp file names per invocation ID - Strict file permissions (0o750 dirs, 0o600 files) MB6: Document stage through hardened safegit - RunContextWithRange uses safegit with exact baseSHA..inputSHA range - No env inheritance, no hook execution, no HEAD reference - Document findings carry Code/Class/Paths for stable fingerprints - Runner passes both BaseSHA and InputSHA to document stage MB7: Real end-to-end tests for every terminal outcome - managed_e2e_test.go drives managed.Run with a real git repo - Uses fakeagent (agenttest.Build) for scripted findings - Covers: passed, needs_decision, failed_retryable, failed_terminal, rerun-with-approved-decision, evidence invocation uniqueness, report-only workspace non-mutation, usage error (exit 2) Additional fixes: - Finding priority: terminal > retryable > needs_decision > passed (prevents needs_decision from overriding failed_retryable) - finding.reported emit errors now propagate as infrastructure_error - ReviewAgentBinaryPath/ExtraEnv threading for test injection - WriteManifest called (best-effort) after WriteTerminal - MadeVersion field in TerminalManifest - Golden fixtures: corrected to 64-hex policy hash (was 72 chars) - Document findings: refactored to shared computeFindings helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Override safe.bareRepository=explicit config in all test contexts that use bare git repositories. Git 2.44+ enforces strict safety requiring explicit configuration for bare repos. Changes: - gitgate: Add bareRepoEnv() helper to set safe.bareRepository=all - All git commands on bare repos now include env override - testhelpers_test: Pass safe.bareRepository config to all git commands - bare.go: Set config during git init --bare - bare_test.go: Override config for verification command This allows tests to work regardless of system-wide git config settings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add GIT_CONFIG overrides to all git command invocations in orchestrator/scaffold.go and orchestrator/workfunc_test.go to allow operations on bare repositories with Git 2.44+ (safe.bareRepository=explicit). - orchestrator/scaffold.go: Add gitEnv() helper and pass to all execpkg.Run() calls - orchestrator/workfunc_test.go: Add git config to cat-file verification command Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add GIT_CONFIG overrides to all git command invocations in pipeline/push and cmd/made packages to allow operations on bare repositories with Git 2.44+ (safe.bareRepository=explicit). - internal/pipeline/push/testhelpers_test.go: Add git config to commitEnv() - cmd/made/gate.go: Add gitEnv() helper and pass to all execpkg.Run() calls - cmd/made/gate_test.go: Add git config to testGit() and testGitOutput() helpers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…daemon Add gitEnv() to remaining exec.Run calls in cmd/made/daemon.go to ensure safe.bareRepository=all config is passed for bare repository operations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add invocation_id to Event struct and include it in every JSON-Lines event. This ensures durable event identity (run_id, invocation_id, sequence) is unique across reruns and allows Consigliere to safely deduplicate and replay events. - Add invocation_id field to Event struct (after run_id) - Add invocation_id field to DecisionsFile struct (informational, not binding) - Update EventWriter.Emit to include opts.InvocationID in every event - Update managed e2e test to capture and log invocation_id - Note: Decisions bind to (run_id, mission_id, input_sha, base_sha, policy_hash) but invocation_id is included for reference and consumed by Consigliere Fixes: Finding identity and event deduplication for reruns. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…blocker 2) Add VerifyExactInputSHA to check that HEAD == input_sha and workspace is clean before and after each validation stage. This prevents undetected concurrent mutations or workspace changes during stage execution. - Add VerifyExactInputSHA function that checks HEAD == inputSHA and status clean - Call VerifyExactInputSHA before each stage starts - Call VerifyExactInputSHA after each stage completes (after mutation check) - Both checks are infrastructure guards with terminal failure This ensures the validation is always run against the exact expected commit, preventing situations where concurrent changes cause different commits to be validated under the same input_sha identity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…_hash (blocker 7) Update all JSONL and JSON fixture files to include invocation_id field and correct policy_hash length (64 hex chars). Fixtures are: - passed.jsonl - needs-decision.jsonl - failed-retryable.jsonl - failed-terminal.jsonl - infrastructure-error.jsonl - decisions-approved.json - decisions-rejected.json Each event now includes invocation_id in the envelope, and run.completed payload includes invocation_id. Decision files now include invocation_id as informational field. Policy hash now uses correct 64-character format. Evidence paths updated to use invocation_id instead of safe-run-id prefix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s (blocker 1) Add checkDuplicateFingerprints method to detect when two or more findings in the same stage have identical fingerprints. Returns infrastructure_error if duplicates are found, preventing ambiguous decision application. Duplicate fingerprints would allow a Decision to approve or reject the wrong finding, or fail to recognize the same finding on rerun. This check ensures finding identity is unambiguous within each stage. Add TestManaged_DuplicateFingerprintDetection to verify collision detection. The test creates two identical findings and verifies infrastructure_error is returned with appropriate error message. This is a partial fix for blocker 1. Remaining work: - Require managed-review findings to include stable fields (code, class, paths) - Document stable finding identity requirements Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…partial) Apply evidence.RedactString to finding descriptions before emitting finding.reported events and before including them in error messages. This prevents sensitive data (API keys, secrets, credentials) from leaking through finding descriptions that are returned to Consigliere. Redaction is applied to: - finding.reported event descriptions (review and document stages) - blocking finding error messages - ask-user finding rejection messages - document finding rejection messages This prevents candidate-controlled data from escaping the validation process. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t (blockers 6, 7) Add comprehensive subprocess isolation section documenting security requirements for Made process execution: - Filesystem isolation: read-only workspace, writable evidence dir only - Environment isolation: no sensitive credentials in process env - Network isolation: no unexpected network access - Privilege isolation: non-root, no capability escalation Recommend Consigliere (Option A) enforces isolation via containerization, not Made. This establishes security boundary before Made invocation. Update evidence directory layout documentation: - Show correct structure with hashed-run-id and invocation-id - Explain why invocation-id exists (reruns share directory) - Update Decisions schema to include invocation_id and base_sha fields - Use concrete values in Decisions example instead of placeholders Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…(blocker 1) CRITICAL FIX: gitEnv() was replacing the entire process environment instead of augmenting it, stripping SSH_AUTH_SOCK, HOME, credential-helper settings, and other authentication context required for credentialed Git operations. Changed gitEnv() in both cmd/made/gate.go and internal/orchestrator/scaffold.go to use append(os.Environ(), ...) instead of returning a bare slice. This preserves inherited credentials and authentication context while adding: - commit.gpgsign=false (disable GPG signing) - safe.bareRepository=all (allow bare repo operations) Fixes standalone Git operations on private SSH and HTTPS remotes that depend on inherited SSH_AUTH_SOCK, .gitconfig, or credential helpers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CRITICAL SECURITY FIX: Managed findings must include stable structural fields (code, class, paths) to prevent Decisions from being applied to wrong findings. Previous behavior: Fingerprints omitted description whenever ANY structural field was present, creating collisions when structural fields were identical but descriptions differed. Example collision (now prevented): - Run 1: kind=ask-user, code='', class='', paths=['auth.go'] description='expire immediately?' → fingerprint A - Run 2: kind=ask-user, code='', class='', paths=['auth.go'] description='encrypt at rest?' → same fingerprint A Decision from run 1 silently applied to run 2 (wrong question) New behavior: Fingerprints are computed ONLY from structural fields (stage, kind, code, class, paths, symbol). Descriptions are completely omitted and only used for human readability. This requires managed findings to provide: - code: stable defect/rule identifier (e.g., 'review.security_issue') - class: stable category (e.g., 'security') - paths: repository-relative affected paths - symbol: stable locus when applicable Missing fields are rejected at preflight with infrastructure_error. Add ValidateStableFindingIdentity to enforce requirements. Apply validation in review and document stages. Add two new e2e tests: 1. TestManaged_ParaphraseStability: Same finding, paraphrased description, approved Decision from run 1 applies in run 2 (fingerprint stable) 2. TestManaged_SameFileDifferentFinding: Two different findings on same file with identical code/class/paths collide and fail closed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CORRECTNESS FIX: Evidence references returned by WriteStageFiles were incomplete. They omitted the safeRunID (hashed run_id) directory component. Previous (broken): Evidence stored at: <evidence-dir>/<safeRunID>/<invocationID>/review/findings.json Reference returned: invocationID/review/findings.json ❌ Cannot resolve without knowing safeRunID calculation New (correct): Evidence stored at: <evidence-dir>/<safeRunID>/<invocationID>/review/findings.json Reference returned: <safeRunID>/<invocationID>/review/findings.json ✅ Can resolve directly: <evidence-dir>/<returned-path> This allows Consigliere to follow event evidence_refs directly without implementing the SHA-256(run_id) hashing logic. Update WriteStageFiles to include safeRunID in returned paths. Update passed.jsonl fixture with correct evidence paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cleanup after blocker 2 (fingerprint collision fix): - Remove unused lineRefPattern, stripLineRefs, and normalizeDescription functions from fingerprint.go (description never used in structural mode) - Remove unused regexp import - Fix trailing whitespace in managed_e2e_test.go All tests passing, golangci-lint clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…cker 4) DOCUMENTATION FIX: Protocol documentation contained multiple inaccuracies that don't match the implementation. Update all event examples and protocol descriptions to be consistent with the current code. Key changes: 1. Event envelope: Add invocation_id and base_sha to all event examples - These fields are now in every event envelope (not optional) - Essential for Consigliere to safely deduplicate and track reruns - Examples now show complete envelope 2. Event timing: Fix run.started emission - Was documented: 'After preflight succeeds' - Correct: 'At process start, before preflight validation begins' - Aligns with actual implementation 3. Fingerprint algorithm: Reflect structural-only mode - Was documented: description used as fallback, line refs stripped - Correct: description completely omitted from fingerprint - Requires all managed findings to have code, class, paths - Provides stability across paraphrasing without ambiguity 4. Finding identity requirements: New section - Explicitly list required fields: code, class, paths - Clarify that missing structural fields cause infrastructure_error - Prevent collision and stability issues 5. Decision binding: Include base_sha - Was missing from Decision example - Now required in binding validation - Must match CLI --base-sha flag 6. Evidence layout: Fix storage paths - Was documented: <run-id>/... - Correct: <hashed-run-id>/<invocation-id>/... - Includes both components so paths are directly resolvable - Shows how hash enables efficient batch review of reruns All documentation changes are in docs/managed-validation-v1.md. Integration reference (docs/managed-validation-integration.md) already correctly describes subprocess isolation requirements. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
FIXTURE VALIDATION: Add test that validates all checked-in JSONL and Decision fixtures are faithful to the protocol contract. Previously, fixtures were treated as documentation examples but were never consumed by production tests. This meant documentation could drift from fixtures without detection. New test (TestFixturesAreValid) validates: JSONL fixtures: ✓ Valid JSON parsing (all lines) ✓ Contiguous sequence numbers (1, 2, 3, ...) ✓ Constant run_id, mission_id, invocation_id, input_sha, base_sha, policy_hash ✓ Correct SHA formats (40-hex for SHAs, sha256:<64-hex> for policy hash) ✓ Invocation_id present on every event ✓ Base_sha present on every event ✓ First event is run.started ✓ Exactly one terminal event (run.completed) Decision fixtures: ✓ Valid JSON parsing ✓ Schema version == 1 ✓ Required fields: run_id, mission_id, input_sha, base_sha, policy_hash, invocation_id ✓ Correct SHA formats ✓ No duplicate decision_id values ✓ Valid fingerprints (sha256:<64-hex>) ✓ Valid outcomes (approved or rejected) ✓ Valid scopes (one_shot, sha_bound, mission_finding_waiver) This ensures: - Fixtures match the implementation contract exactly - Documentation examples are enforced by tests - Hash format inconsistencies are caught immediately - Future fixture changes are validated automatically Also fixes infrastructure-error.jsonl fixture which was missing run.started event. Per updated protocol documentation, run.started is emitted at process start before preflight validation begins. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use nolint pattern for file.Close defer - Fix variable formatting/alignment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The 'made' binary was accidentally added to git history in an earlier commit. Remove it from the index and prevent future commits of built binaries by adding /made to .gitignore. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CORRECTNESS FIX: Managed findings must provide safe, repository-relative paths. Paths can be abused to escape containment or expose structure. Enhanced ValidateStableFindingIdentity to reject: - Absolute paths (must be repository-relative) - Paths with '..' (cannot escape working directory) - Unclean paths with '.' or redundant separators - Empty paths Also clarified in documentation that 'code' field must be finding-specific (e.g., 'review.sql_injection'), not generic category names (e.g., 'review.issue'). This prevents reuse of the same fingerprint for completely different findings on the same file. Added 4 new tests: - TestValidateFindingPathsRejectAbsolute - TestValidateFindingPathsRejectEscape - TestValidateFindingPathsRejectUnclean - TestValidateFindingCodeMustBeSpecific All tests passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CONTAINMENT FIX: Evidence directories can be symlinks that point into the workspace, bypassing containment checks. Changes: 1. Canonicalize evidence directory through EvalSymlinks - If evidence-dir exists, resolve it fully - If it doesn't exist, resolve nearest existing parent - Reject overlap with canonical workspace path 2. Stricter path validation for findings - Reject absolute paths (must be repository-relative) - Reject paths with '..' escape sequences - Reject unclean paths (redundant separators, '.' components) - Provide clear error messages showing forbidden patterns This prevents: - Symlink-based escape from evidence containment - Absolute path disclosure of workspace structure - Path traversal attacks using '..' All preflight tests passing (TestPreflight_* suite). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SUMMARY CONSISTENCY FIX: manifest.json and terminal.json could report contradictory outcomes. The run built a manifest with the original outcome, then tried to write it as terminal.json. If that write failed, the outcome changed to infrastructure_error but the manifest retained the previous outcome, causing disagreement between files. Changes: 1. Removed WriteManifest() method from ManagedEvidenceStore 2. Removed WriteManifest call from run.go 3. terminal.json is now the single authoritative summary file 4. Comments clarify this is the sole summary This guarantees: - One authoritative outcome in terminal.json - No contradictory summary files - If write fails, outcome changes before building the summary All tests passing (23 managed tests + integration suite). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ty requirements (blocker 2) MANAGED REVIEW SCHEMA FIX: Codex schema still treated code, class, and symbol as optional, but managed mode requires them for stable Decision binding. Real agents could produce valid JSON per the old schema but fail managed validation. Changes: 1. NewManagedReviewTask() - New review task builder for managed mode - Requires finding-specific code (not generic 'security') - Requires class from stable taxonomy - Requires repository-relative, normalized paths - Requires symbol/locus for multi-finding files - Includes detailed requirements in prompt 2. resolveReviewTask() updated to use NewManagedReviewTask - When ReportOnly is true (managed mode), use strict contract - Standalone review continues using NewReviewTask 3. Test coverage for managed review contract - Verifies mode marker, finding-specific requirement - Verifies code, class, path requirements - Verifies contract marker is valid JSON This prevents fingerprint collisions across runs when findings are paraphrased but have same structural fields (code, class, paths). Tests passing: - agent reviewcontract tests (3 passing, +1 managed) - managed mode end-to-end tests (all 23 passing) - review pipeline tests (all passing) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix formatting errors identified by golangci-lint: - internal/agent/reviewcontract_test.go: Remove trailing newline - internal/managed/fingerprint.go: Fix comment indentation Formatting now complies with gofmt standards. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Made Managed Validation V1
Protocol version: 1
Schema version: 1
CLI:
made validate --managed --json-eventsSummary
Adds an additive, short-lived, daemonless managed-validation mode to Made. Consigliere invokes it to validate an immutable input commit SHA. Made validates and exits. Consigliere orchestrates everything else.
Checklist
Protocol version: 1
Schema version: 1
CLI:
made validate --managed --json-events --run-id ... --mission-id ... --workspace ... --base-sha ... --input-sha ... --trusted-config ... --policy-hash ... --evidence-dir ... [--decisions ...]Managed stages: review (report-only), test, document, lint
Terminal outcomes: passed, needs_decision, failed_retryable, failed_terminal, infrastructure_error, canceled
Exit codes: 0=passed, 1=infrastructure_error, 2=usage/contract error, 3=needs_decision, 4=failed_retryable, 5=failed_terminal, 130=canceled
Compatibility impact: None. All existing commands unchanged. Standalone auto-fix retained. Daemon untouched.
Consigliere changes required: None — integration uses only the public command and docs.
Consigliere changes included in this PR: None.
Known limitations: Review stage requires a configured review agent; without one it returns infrastructure_error (the agent binary is not present in test environments).
What this adds
New command
New packages
internal/managed: contract types, versioned JSON-Lines event writer, preflight (15 checks), Decisions parser, stable fingerprints, evidence store, stage runner, lifecycleinternal/safegit: hardened Git execution that strips all GIT_* env vars, core.hooksPath, fsmonitor, credential helpers — shared across managed modeModified (additive only)
internal/pipeline/review: addReportOnly boolto Options — standalone callers unaffectedinternal/pipeline/document: addRunContextWithBaseSHAfor exact-SHA diff rangesinternal/agent/findings: additiveCode,Class,Symbolfields (optional, omitempty)internal/config: addParseBytesfor hash-verified config loadingcmd/made: addvalidatecommand; addvalidate.managed.v1to capabilitiesDocumentation
docs/managed-validation-v1.md: full contract document (17 sections, updated for accuracy)docs/managed-validation-integration.md: machine-consumer integration referenceGolden fixtures
internal/managed/testdata/: passed, needs-decision, failed-retryable, failed-terminal, infrastructure-error, decisions-approved/rejected (all validated by tests)Merge blockers fixed in this update
Validation evidence
✅
go build ./...: clean✅
go test ./...: all packages passing✅
go test -race -shuffle=on -count=1 ./...: all passing (no race conditions)✅
go vet ./...: clean✅
golangci-lint run ./...: 0 issues✅ CI workflow: SUCCESS on latest push
✅ 13 managed-mode tests passing (paraphrase stability, collision detection, e2e outcomes)
Commits
All blockers from the verdict are addressed. PR ready for merge review.