From 775bd905ac4a65a01616e10d37af790cc67faa13 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 30 May 2026 16:20:33 +0000 Subject: [PATCH 1/5] Initial plan From 41a7bc19f288fb3c76c848f741c42d3760179762 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 30 May 2026 16:35:42 +0000 Subject: [PATCH 2/5] Add SPDD spec updates and manifest/version guard coverage Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .../reference/model-alias-specification.md | 10 +- ...pository-package-manifest-specification.md | 61 +++++++++++- .../reference/safe-outputs-specification.md | 8 ++ pkg/cli/add_package_manifest.go | 50 +++++++++- pkg/cli/add_package_manifest_test.go | 97 +++++++++++++++++++ pkg/cli/compile_repository_manifest_test.go | 36 +++++++ pkg/parser/schemas/aw_manifest_schema.json | 39 ++++++++ .../tools_guard_policy_integration_test.go | 50 ++++++++++ ...github-mcp-access-control-specification.md | 10 ++ scratchpad/guard-policies-specification.md | 30 ++++-- 10 files changed, 377 insertions(+), 14 deletions(-) create mode 100644 pkg/workflow/tools_guard_policy_integration_test.go diff --git a/docs/src/content/docs/reference/model-alias-specification.md b/docs/src/content/docs/reference/model-alias-specification.md index d90a377d144..3e641c93d71 100644 --- a/docs/src/content/docs/reference/model-alias-specification.md +++ b/docs/src/content/docs/reference/model-alias-specification.md @@ -704,7 +704,8 @@ At compile time, an implementation SHOULD: | Requirement | Test ID | Level | Status | |-------------|---------|-------|--------| | Bare identifier parsing | T-MAF-001 | 1 | Required | -| Parameter parsing | T-MAF-002, 004 | 1 | Required | +| Provider/model identifier parsing | T-MAF-003 | 1 | Required | +| Parameter parsing | T-MAF-002, T-MAF-004 | 1 | Required | | Glob rejection in engine.model | T-MAF-005 | 1 | Required | | Invalid effort value rejection | T-MAF-006 | 1 | Required | | Temperature range validation | T-MAF-007 | 1 | Required | @@ -714,10 +715,15 @@ At compile time, an implementation SHOULD: | Transitive alias resolution | T-MAF-021 | 2 | Required | | Parameter propagation | T-MAF-022 | 2 | Required | | Caller-wins parameter merge | T-MAF-023 | 2 | Required | +| Builtin-alias composition | T-MAF-024 | 2 | Required | | Default policy (`""`) | T-MAF-025 | 2 | Required | | Semver-aware glob selection (latest wins) | T-MAF-026 | 2 | Required | | Date-suffix tiebreaker | T-MAF-027 | 2 | Required | +| Unversioned model ranking fallback | T-MAF-028 | 2 | Required | | Main workflow wins merge | T-MAF-030 | 2 | Required | +| First import wins on duplicate key | T-MAF-031 | 2 | Required | +| Main workflow overrides import keys | T-MAF-032 | 2 | Required | +| Builtin-only key retention | T-MAF-033 | 2 | Required | | Compile-time circular alias detection | T-MAF-040, 041 | 3 | Required | | Runtime circular alias guard | T-MAF-042 | 3 | Required | | Unrecognized param warning | T-MAF-043 | 3 | Recommended | @@ -852,6 +858,8 @@ Model parameters are compile-time configuration values and are not derived from - **Enhanced**: Compile-time cycle detection (§8.6.1): expanded from a single sentence to a full DFS algorithm with error-message requirements. - **Added**: Models payload merge algorithm pseudocode (§10.2) making the three-layer merge semantics explicit. - **Added**: Merge precedence test T-MAF-033 (builtin-only keys are preserved). +- **Clarified**: `?effort=` parsing/validation semantics now explicitly align with runtime parser behavior (`low|medium|high`, caller-wins merge, compile-time validation). +- **Clarified**: `?temperature=` parsing/validation semantics now explicitly align with runtime parser behavior (numeric range `0.0..2.0`, forwarding and overwrite rules). ### Version 1.0.0 (Draft) diff --git a/docs/src/content/docs/reference/repository-package-manifest-specification.md b/docs/src/content/docs/reference/repository-package-manifest-specification.md index ca7d5cdce0e..82f000a31d3 100644 --- a/docs/src/content/docs/reference/repository-package-manifest-specification.md +++ b/docs/src/content/docs/reference/repository-package-manifest-specification.md @@ -43,9 +43,13 @@ The manifest document MUST be a YAML mapping. Unknown top-level fields MUST be r | --- | --- | --- | --- | | `manifest-version` | string | No | Manifest format version. Defaults to `"1"`. | | `min-version` | string | No | Minimum supported `gh-aw` version. | +| `max-version` | string | No | Maximum supported `gh-aw` version. | | `name` | string | Yes | Human-readable package name. | | `emoji` | string | No | Optional package emoji for display in package metadata. | | `description` | string | No | Human-readable package description. | +| `license` | string | No | SPDX license identifier for package metadata. | +| `tags` | array of strings | No | Package tags (max 10 entries, each max 32 chars). | +| `categories` | array of enum strings | No | Package categories (`automation`, `ci-cd`, `code-review`, `security`, `documentation`, `release`, `triage`, `testing`). | | `files` | array of strings | No | Explicit installable workflow file list. | ### 4.2 `manifest-version` @@ -54,14 +58,18 @@ If omitted, `manifest-version` defaults to `"1"`. For this version of the format, the only valid value is `"1"`. -### 4.3 `min-version` +### 4.3 Version constraints: `min-version` and `max-version` -If present, `min-version` MUST use the exact `vMAJOR.minor.patch` form, such as: +If present, `min-version` and `max-version` MUST use the exact `vMAJOR.minor.patch` form, such as: - `v1.2.3` If the running compiler version is lower than `min-version`, validation MUST fail. +If the running compiler version is higher than `max-version`, validation MUST fail. + +If both `min-version` and `max-version` are present, `min-version` MUST be less than or equal to `max-version`. + ### 4.4 `name` `name` MUST be present and MUST be a non-empty string after trimming surrounding whitespace. @@ -87,6 +95,29 @@ Each entry MUST be resolved relative to the package root and MUST match one of t Duplicate entries SHOULD be ignored after normalization. +### 4.8 `license` + +If present, `license` MUST be a valid SPDX identifier string. + +### 4.9 `tags` + +If present, `tags` MUST be an array of strings with at most 10 entries. + +Each `tags` entry MUST be 1-32 characters. + +### 4.10 `categories` + +If present, `categories` MUST be an array of enum values from this set: + +- `automation` +- `ci-cd` +- `code-review` +- `security` +- `documentation` +- `release` +- `triage` +- `testing` + ## 5. Installable file resolution Supported installable paths are: @@ -131,7 +162,13 @@ Validation MUST fail for at least the following conditions: - missing or empty `name`; - unsupported `manifest-version`; - invalid `min-version`; +- invalid `max-version`; - current compiler version is lower than `min-version`; +- current compiler version is higher than `max-version`; +- `min-version` greater than `max-version`; +- invalid `license` (non-SPDX); +- more than 10 `tags` entries or any tag longer than 32 characters; +- invalid `categories` value; - unknown top-level fields, including `docs`; or - missing required `README.md`; or - no installable workflow files resolved. @@ -196,3 +233,23 @@ Documentation file: ```text packages/repo-assist/README.md ``` + +## 10. Package Lifecycle + +### 10.1 `gh aw add` + +- The implementation MUST fetch and validate `aw.yml` before installing package files. +- The implementation MUST reject installation when manifest validation returns any `manifest_error`. +- The implementation MUST copy raw `.yml` workflow includes verbatim without compilation. + +### 10.2 `gh aw update` + +- The implementation MUST re-fetch and re-validate `aw.yml` during update. +- The implementation MUST preserve local install targets and only replace files belonging to the package manifest selection. +- The implementation MUST fail the update if a newly resolved package manifest is incompatible with current compiler version constraints. + +### 10.3 `gh aw remove` + +- The implementation MUST remove only files that were previously installed from the package. +- The implementation MUST NOT delete user-authored files outside the tracked package install set. +- The implementation MUST report missing installed files as warnings (not hard failures) and continue removal for remaining tracked files. diff --git a/docs/src/content/docs/reference/safe-outputs-specification.md b/docs/src/content/docs/reference/safe-outputs-specification.md index f97c8febf6f..53180e9c18e 100644 --- a/docs/src/content/docs/reference/safe-outputs-specification.md +++ b/docs/src/content/docs/reference/safe-outputs-specification.md @@ -4227,6 +4227,14 @@ All errors MUST be logged to: ## 10. Execution Guarantees + + ### 10.1 Atomicity **Single-Item Operations**: Complete success or complete failure (no partial state). diff --git a/pkg/cli/add_package_manifest.go b/pkg/cli/add_package_manifest.go index 941e9a3ce61..bd421025eda 100644 --- a/pkg/cli/add_package_manifest.go +++ b/pkg/cli/add_package_manifest.go @@ -172,9 +172,13 @@ func loadRepositoryPackageManifestFile(owner, repo, packagePath, ref, host strin type repositoryPackageManifest struct { ManifestVersion string MinVersion string + MaxVersion string Name string Emoji string Description string + License string + Tags []string + Categories []string Includes []string Files []string Skills []string // skill directory paths (e.g. "skills/my-skill") @@ -219,13 +223,27 @@ func parseRepositoryPackageManifest(manifestPath string, content []byte) (*repos if !isSupportedManifestMinVersion(manifest.MinVersion) { return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version must use vMAJOR.minor.patch, got %q", manifestPath, minVersion) } + } + if maxVersion, ok := stringValue(root["max-version"]); ok { + manifest.MaxVersion = strings.TrimSpace(maxVersion) + if !isSupportedManifestMinVersion(manifest.MaxVersion) { + return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: max-version must use vMAJOR.minor.patch, got %q", manifestPath, maxVersion) + } + } + if manifest.MinVersion != "" && manifest.MaxVersion != "" && semverutil.Compare(manifest.MinVersion, manifest.MaxVersion) > 0 { + return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version %q cannot be greater than max-version %q", manifestPath, manifest.MinVersion, manifest.MaxVersion) + } + if manifest.MinVersion != "" || manifest.MaxVersion != "" { currentVersion := GetVersion() if !semverutil.IsValid(currentVersion) { - return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version validation requires a semantic-versioned compiler, but the current compiler version %q is not a valid semantic version (this indicates a build issue)", manifestPath, currentVersion) + return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: version-range validation requires a semantic-versioned compiler, but the current compiler version %q is not a valid semantic version (this indicates a build issue)", manifestPath, currentVersion) } - if semverutil.Compare(currentVersion, manifest.MinVersion) < 0 { + if manifest.MinVersion != "" && semverutil.Compare(currentVersion, manifest.MinVersion) < 0 { return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version %q requires gh-aw %s or newer (current: %s)", manifestPath, manifest.MinVersion, manifest.MinVersion, currentVersion) } + if manifest.MaxVersion != "" && semverutil.Compare(currentVersion, manifest.MaxVersion) > 0 { + return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: max-version %q requires gh-aw %s or older (current: %s)", manifestPath, manifest.MaxVersion, manifest.MaxVersion, currentVersion) + } } if description, ok := stringValue(root["description"]); ok { @@ -238,6 +256,15 @@ func parseRepositoryPackageManifest(manifestPath string, content []byte) (*repos if emoji, ok := stringValue(root["emoji"]); ok { manifest.Emoji = emoji } + if license, ok := stringValue(root["license"]); ok { + manifest.License = strings.TrimSpace(license) + } + if tags, ok := stringSliceValue(root["tags"]); ok { + manifest.Tags = tags + } + if categories, ok := stringSliceValue(root["categories"]); ok { + manifest.Categories = categories + } if includesValue, ok := root["includes"]; ok { includes, includeWarnings := extractManifestIncludes(includesValue, manifestPath) @@ -749,6 +776,25 @@ func stringValue(value any) (string, bool) { return s, ok } +func stringSliceValue(value any) ([]string, bool) { + switch raw := value.(type) { + case []any: + out := make([]string, 0, len(raw)) + for _, entry := range raw { + s, ok := entry.(string) + if !ok { + return nil, false + } + out = append(out, s) + } + return out, true + case []string: + return append([]string(nil), raw...), true + default: + return nil, false + } +} + func isRepositoryFileNotFound(err error) bool { return errors.Is(err, errRepositoryPackageFileNotFound) } diff --git a/pkg/cli/add_package_manifest_test.go b/pkg/cli/add_package_manifest_test.go index 795f4a5561f..3bb872d01e2 100644 --- a/pkg/cli/add_package_manifest_test.go +++ b/pkg/cli/add_package_manifest_test.go @@ -270,6 +270,31 @@ files: assert.Equal(t, []string{"workflows/review.md"}, pkg.InstallationSource) }) + t.Run("accepts compatible max-version", func(t *testing.T) { + downloadPackageFileFromGitHubForHost = func(owner, repo, path, ref, host string) ([]byte, error) { + switch path { + case "aw.yml": + return []byte(`max-version: v1.2.3 +name: Repo Assist +files: + - workflows/review.md +`), nil + case "README.md": + return []byte("# Repo Assist\n"), nil + default: + return nil, createRepositoryPackageNotFoundError(path) + } + } + listPackageWorkflowFilesForHost = func(owner, repo, ref, workflowPath, host string) ([]string, error) { + t.Fatalf("unexpected scan of %s", workflowPath) + return nil, nil + } + + pkg, err := resolveRepositoryPackage(&RepoSpec{RepoSlug: "owner/repo"}, "") + require.NoError(t, err) + assert.Equal(t, []string{"workflows/review.md"}, pkg.InstallationSource) + }) + t.Run("rejects unsupported manifest-version", func(t *testing.T) { downloadPackageFileFromGitHubForHost = func(owner, repo, path, ref, host string) ([]byte, error) { if path == "aw.yml" { @@ -331,6 +356,78 @@ name: Repo Assist assert.Contains(t, err.Error(), `requires gh-aw`) }) + t.Run("rejects incompatible max-version", func(t *testing.T) { + downloadPackageFileFromGitHubForHost = func(owner, repo, path, ref, host string) ([]byte, error) { + if path == "aw.yml" { + return []byte(`max-version: v1.0.0 +name: Repo Assist +`), nil + } + return nil, createRepositoryPackageNotFoundError(path) + } + + _, err := resolveRepositoryPackage(&RepoSpec{RepoSlug: "owner/repo"}, "") + require.Error(t, err) + assert.Contains(t, err.Error(), `or older`) + }) + + t.Run("rejects manifest when min-version is greater than max-version", func(t *testing.T) { + downloadPackageFileFromGitHubForHost = func(owner, repo, path, ref, host string) ([]byte, error) { + if path == "aw.yml" { + return []byte(`min-version: v1.3.0 +max-version: v1.2.0 +name: Repo Assist +`), nil + } + return nil, createRepositoryPackageNotFoundError(path) + } + + _, err := resolveRepositoryPackage(&RepoSpec{RepoSlug: "owner/repo"}, "") + require.Error(t, err) + assert.Contains(t, err.Error(), `cannot be greater`) + }) + + t.Run("accepts license tags and categories metadata", func(t *testing.T) { + downloadPackageFileFromGitHubForHost = func(owner, repo, path, ref, host string) ([]byte, error) { + switch path { + case "aw.yml": + return []byte(`name: Repo Assist +license: MIT +tags: [triage, issues] +categories: [automation, triage] +files: + - workflows/review.md +`), nil + case "README.md": + return []byte("# Repo Assist\n"), nil + default: + return nil, createRepositoryPackageNotFoundError(path) + } + } + listPackageWorkflowFilesForHost = func(owner, repo, ref, workflowPath, host string) ([]string, error) { + t.Fatalf("unexpected scan of %s", workflowPath) + return nil, nil + } + + _, err := resolveRepositoryPackage(&RepoSpec{RepoSlug: "owner/repo"}, "") + require.NoError(t, err) + }) + + t.Run("rejects invalid categories metadata", func(t *testing.T) { + downloadPackageFileFromGitHubForHost = func(owner, repo, path, ref, host string) ([]byte, error) { + if path == "aw.yml" { + return []byte(`name: Repo Assist +categories: [unknown] +`), nil + } + return nil, createRepositoryPackageNotFoundError(path) + } + + _, err := resolveRepositoryPackage(&RepoSpec{RepoSlug: "owner/repo"}, "") + require.Error(t, err) + assert.Contains(t, err.Error(), `categories`) + }) + t.Run("requires package README", func(t *testing.T) { downloadPackageFileFromGitHubForHost = func(owner, repo, path, ref, host string) ([]byte, error) { if path == "aw.yml" { diff --git a/pkg/cli/compile_repository_manifest_test.go b/pkg/cli/compile_repository_manifest_test.go index 28cf43c8737..bd782b7b69b 100644 --- a/pkg/cli/compile_repository_manifest_test.go +++ b/pkg/cli/compile_repository_manifest_test.go @@ -53,6 +53,42 @@ name: Repo Assist assert.Contains(t, err.Error(), `requires gh-aw`) } +func TestCompileWorkflows_ValidatesRootAwManifestMaxVersion(t *testing.T) { + tmpDir := testutil.TempDir(t, "aw-manifest-max-version-*") + originalWd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(originalWd) }) + require.NoError(t, os.Chdir(tmpDir)) + + cmd := exec.Command("git", "init") + cmd.Dir = tmpDir + require.NoError(t, cmd.Run()) + + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".github", "workflows"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, ".github", "workflows", "test.md"), []byte(`--- +on: workflow_dispatch +permissions: + contents: read +engine: copilot +--- + +# Test +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README.md"), []byte("# Repo Assist\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "aw.yml"), []byte(`manifest-version: "1" +max-version: v1.0.0 +name: Repo Assist +`), 0o644)) + + originalVersion := GetVersion() + SetVersionInfo("v1.2.3") + t.Cleanup(func() { SetVersionInfo(originalVersion) }) + + _, err = CompileWorkflows(context.Background(), CompileConfig{}) + require.Error(t, err) + assert.Contains(t, err.Error(), `or older`) +} + func TestCompileWorkflows_JSONOutputIncludesManifestValidationResult(t *testing.T) { tmpDir := testutil.TempDir(t, "aw-manifest-json-*") originalWd, err := os.Getwd() diff --git a/pkg/parser/schemas/aw_manifest_schema.json b/pkg/parser/schemas/aw_manifest_schema.json index 17cf8b01883..0b9e67ae0ed 100644 --- a/pkg/parser/schemas/aw_manifest_schema.json +++ b/pkg/parser/schemas/aw_manifest_schema.json @@ -13,6 +13,11 @@ "description": "Minimum supported gh-aw CLI version. Requires the exact vMAJOR.minor.patch form.", "pattern": "^v[0-9]+\\.[0-9]+\\.[0-9]+$" }, + "max-version": { + "type": "string", + "description": "Maximum supported gh-aw CLI version. Requires the exact vMAJOR.minor.patch form.", + "pattern": "^v[0-9]+\\.[0-9]+\\.[0-9]+$" + }, "name": { "type": "string", "minLength": 1 @@ -23,6 +28,40 @@ "description": { "type": "string" }, + "license": { + "type": "string", + "description": "SPDX license identifier for the package.", + "minLength": 1, + "maxLength": 64, + "pattern": "^[A-Za-z0-9.+-]+$" + }, + "tags": { + "type": "array", + "description": "Up to 10 short package tags, each at most 32 characters.", + "maxItems": 10, + "items": { + "type": "string", + "minLength": 1, + "maxLength": 32 + } + }, + "categories": { + "type": "array", + "description": "Package category classification.", + "items": { + "type": "string", + "enum": [ + "automation", + "ci-cd", + "code-review", + "security", + "documentation", + "release", + "triage", + "testing" + ] + } + }, "includes": { "type": "array", "description": "Installable package entries. Use folder naming conventions to infer type: workflows under workflows/, agentic-workflows/, or .github/workflows/; skill directories under skills/ or .github/skills/; agent files under agents/ or .github/agents/.", diff --git a/pkg/workflow/tools_guard_policy_integration_test.go b/pkg/workflow/tools_guard_policy_integration_test.go new file mode 100644 index 00000000000..7bfbd225da8 --- /dev/null +++ b/pkg/workflow/tools_guard_policy_integration_test.go @@ -0,0 +1,50 @@ +//go:build !integration + +package workflow + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGuardPolicyEndToEndCompilation(t *testing.T) { + workflowContent := `--- +on: + workflow_dispatch: +permissions: + contents: read +engine: copilot +tools: + github: + allowed-repos: + - github/* + - github/gh-aw + min-integrity: approved +--- + +# Guard Policy Integration +` + + tmpDir := t.TempDir() + workflowPath := filepath.Join(tmpDir, "guard-policy.md") + require.NoError(t, os.WriteFile(workflowPath, []byte(workflowContent), 0o644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(workflowPath)) + + lockPath := filepath.Join(tmpDir, "guard-policy.lock.yml") + lockBytes, err := os.ReadFile(lockPath) + require.NoError(t, err) + + lockContent := string(lockBytes) + assert.Contains(t, lockContent, `"guard-policies"`) + assert.Contains(t, lockContent, `"allow-only"`) + assert.Contains(t, lockContent, `"min-integrity": "approved"`) + assert.Contains(t, lockContent, `"repos": [`) + assert.Contains(t, lockContent, `"github/*"`) + assert.Contains(t, lockContent, `"github/gh-aw"`) +} diff --git a/scratchpad/github-mcp-access-control-specification.md b/scratchpad/github-mcp-access-control-specification.md index 98c0d2fb8b2..9400c495cbc 100644 --- a/scratchpad/github-mcp-access-control-specification.md +++ b/scratchpad/github-mcp-access-control-specification.md @@ -1811,6 +1811,16 @@ Implementations MUST log all access control decisions with the following informa - Error messages SHOULD be generic: "Access denied" rather than "Repository is private" - Detailed access denials logged separately for administrators +### 9.5 Configuration Reload and Stale-Config Safeguards + +Implementations that support runtime config reload MUST enforce the following safeguards: + +- Reloaded access-control configuration MUST be schema-validated before activation. +- If reload validation fails, the gateway MUST keep the last known-good access-control configuration active. +- The gateway MUST atomically swap access-control config to avoid partial state between concurrent requests. +- The gateway MUST emit a structured stale-config event when a reload attempt is rejected, including reason and source revision/hash. +- Requests evaluated while config state is unknown or inconsistent MUST fail closed (deny) until a valid configuration is restored. + --- ## 10. Integration with MCP Gateway diff --git a/scratchpad/guard-policies-specification.md b/scratchpad/guard-policies-specification.md index 301b8a8d90f..325846db316 100644 --- a/scratchpad/guard-policies-specification.md +++ b/scratchpad/guard-policies-specification.md @@ -316,10 +316,11 @@ tools: - `TestValidateReposScopeWithStringSlice`: 4 cases covering `[]string` and `[]any` input types - Tests live in `pkg/workflow/tools_validation_test.go` -2. **Integration Tests** (Pending): - - Test end-to-end workflow compilation with guard policies - - Test that guard policies appear in compiled workflow YAML - - Test that guard policies are passed to MCP gateway configuration +2. **Integration Tests** (Complete): + - `TestGuardPolicyEndToEndCompilation` verifies end-to-end workflow compilation with guard policies + - Confirms guard policies appear in compiled lock workflow YAML + - Confirms allow-only policy includes expected `repos` and `min-integrity` values + - Test file: `pkg/workflow/tools_guard_policy_integration_test.go` ## Next Steps @@ -348,12 +349,23 @@ tools: 5. **Clarity**: Clear error messages and validation 6. **Documentation**: Self-documenting through type system -## Open Questions +## Decisions -1. Should we support negative patterns (e.g., exclude certain repos)? -2. Should we support combining multiple policies (AND/OR logic)? -3. How should conflicts between lockdown and guard policies be resolved? -4. Should we add a "dry-run" mode to test policies before enforcement? +1. **Q1: Negative patterns (exclude repos)** + - **Decision**: **Deferred** + - **Reason**: The current allow-list model is simpler and already supports least-privilege defaults. Negative patterns increase matcher complexity and ambiguity when mixed with wildcards. + +2. **Q2: Combining multiple policies (AND/OR logic)** + - **Decision**: **Out-of-scope** + - **Reason**: v1 keeps a single deterministic policy shape per server. Multi-policy boolean composition requires a separate policy language and evaluator contract. + +3. **Q3: Lockdown and guard-policy conflicts** + - **Decision**: **Accepted** + - **Reason**: Apply most-restrictive-wins semantics. When lockdown and guard policies both apply, the effective scope MUST be the intersection of both constraints. + +4. **Q4: Dry-run mode before enforcement** + - **Decision**: **Deferred** + - **Reason**: Runtime enforcement wiring is still pending. Dry-run will be reconsidered once enforcement emits stable decision logs and telemetry suitable for simulation. ## Conclusion From 797d11524f91e07cc1da4ecd424cf94958397a4d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 30 May 2026 16:37:17 +0000 Subject: [PATCH 3/5] Address review feedback for manifest helpers Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/add_package_manifest.go | 6 ++--- pkg/cli/add_package_manifest_test.go | 36 ++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/pkg/cli/add_package_manifest.go b/pkg/cli/add_package_manifest.go index bd421025eda..dd1b9e57179 100644 --- a/pkg/cli/add_package_manifest.go +++ b/pkg/cli/add_package_manifest.go @@ -220,13 +220,13 @@ func parseRepositoryPackageManifest(manifestPath string, content []byte) (*repos if minVersion, ok := stringValue(root["min-version"]); ok { manifest.MinVersion = strings.TrimSpace(minVersion) - if !isSupportedManifestMinVersion(manifest.MinVersion) { + if !isSupportedManifestVersion(manifest.MinVersion) { return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version must use vMAJOR.minor.patch, got %q", manifestPath, minVersion) } } if maxVersion, ok := stringValue(root["max-version"]); ok { manifest.MaxVersion = strings.TrimSpace(maxVersion) - if !isSupportedManifestMinVersion(manifest.MaxVersion) { + if !isSupportedManifestVersion(manifest.MaxVersion) { return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: max-version must use vMAJOR.minor.patch, got %q", manifestPath, maxVersion) } } @@ -803,7 +803,7 @@ func isRepositoryPackageManifestNotFound(err error) bool { return errors.Is(err, errRepositoryPackageManifestNotFound) } -func isSupportedManifestMinVersion(version string) bool { +func isSupportedManifestVersion(version string) bool { const expectedManifestMinVersionDotCount = 2 return semverutil.IsActionVersionTag(version) && strings.Count(strings.TrimPrefix(version, "v"), ".") == expectedManifestMinVersionDotCount } diff --git a/pkg/cli/add_package_manifest_test.go b/pkg/cli/add_package_manifest_test.go index 3bb872d01e2..3af8d3762ef 100644 --- a/pkg/cli/add_package_manifest_test.go +++ b/pkg/cli/add_package_manifest_test.go @@ -1483,3 +1483,39 @@ files: assert.True(t, agent.IsPackageAgentFile) assert.Equal(t, agentMD, agent.Content) } + +func TestStringSliceValue(t *testing.T) { + t.Run("accepts []string", func(t *testing.T) { + got, ok := stringSliceValue([]string{"a", "b"}) + require.True(t, ok) + assert.Equal(t, []string{"a", "b"}, got) + }) + + t.Run("accepts []any with strings", func(t *testing.T) { + got, ok := stringSliceValue([]any{"a", "b"}) + require.True(t, ok) + assert.Equal(t, []string{"a", "b"}, got) + }) + + t.Run("rejects []any with non-string values", func(t *testing.T) { + _, ok := stringSliceValue([]any{"a", 1}) + assert.False(t, ok) + }) + + t.Run("accepts empty slices", func(t *testing.T) { + gotAny, okAny := stringSliceValue([]any{}) + require.True(t, okAny) + assert.Empty(t, gotAny) + + gotString, okString := stringSliceValue([]string{}) + require.True(t, okString) + assert.Empty(t, gotString) + }) + + t.Run("rejects nil and non-slice values", func(t *testing.T) { + _, ok := stringSliceValue(nil) + assert.False(t, ok) + _, ok = stringSliceValue("not-a-slice") + assert.False(t, ok) + }) +} From fc193f3b2dc152e4641d6c86dd67f2e22c2033b1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 30 May 2026 16:38:25 +0000 Subject: [PATCH 4/5] Refine manifest version helper naming and tests Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/add_package_manifest.go | 6 +++--- pkg/cli/add_package_manifest_test.go | 5 ++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/cli/add_package_manifest.go b/pkg/cli/add_package_manifest.go index dd1b9e57179..9443eaa8afc 100644 --- a/pkg/cli/add_package_manifest.go +++ b/pkg/cli/add_package_manifest.go @@ -220,13 +220,13 @@ func parseRepositoryPackageManifest(manifestPath string, content []byte) (*repos if minVersion, ok := stringValue(root["min-version"]); ok { manifest.MinVersion = strings.TrimSpace(minVersion) - if !isSupportedManifestVersion(manifest.MinVersion) { + if !isValidManifestVersionFormat(manifest.MinVersion) { return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: min-version must use vMAJOR.minor.patch, got %q", manifestPath, minVersion) } } if maxVersion, ok := stringValue(root["max-version"]); ok { manifest.MaxVersion = strings.TrimSpace(maxVersion) - if !isSupportedManifestVersion(manifest.MaxVersion) { + if !isValidManifestVersionFormat(manifest.MaxVersion) { return nil, nil, fmt.Errorf("invalid Agentic Workflow manifest %q: max-version must use vMAJOR.minor.patch, got %q", manifestPath, maxVersion) } } @@ -803,7 +803,7 @@ func isRepositoryPackageManifestNotFound(err error) bool { return errors.Is(err, errRepositoryPackageManifestNotFound) } -func isSupportedManifestVersion(version string) bool { +func isValidManifestVersionFormat(version string) bool { const expectedManifestMinVersionDotCount = 2 return semverutil.IsActionVersionTag(version) && strings.Count(strings.TrimPrefix(version, "v"), ".") == expectedManifestMinVersionDotCount } diff --git a/pkg/cli/add_package_manifest_test.go b/pkg/cli/add_package_manifest_test.go index 3af8d3762ef..d95c231c891 100644 --- a/pkg/cli/add_package_manifest_test.go +++ b/pkg/cli/add_package_manifest_test.go @@ -1486,9 +1486,12 @@ files: func TestStringSliceValue(t *testing.T) { t.Run("accepts []string", func(t *testing.T) { - got, ok := stringSliceValue([]string{"a", "b"}) + input := []string{"a", "b"} + got, ok := stringSliceValue(input) require.True(t, ok) assert.Equal(t, []string{"a", "b"}, got) + got[0] = "changed" + assert.Equal(t, "a", input[0], "returned slice should be a defensive copy") }) t.Run("accepts []any with strings", func(t *testing.T) { From 3b0cd46259ba8b71c9dc1bd82675911b4de53c1f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 30 May 2026 16:39:23 +0000 Subject: [PATCH 5/5] Document manifest parsing helper behavior Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/add_package_manifest.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/cli/add_package_manifest.go b/pkg/cli/add_package_manifest.go index 9443eaa8afc..91b8534e5f7 100644 --- a/pkg/cli/add_package_manifest.go +++ b/pkg/cli/add_package_manifest.go @@ -776,6 +776,10 @@ func stringValue(value any) (string, bool) { return s, ok } +// stringSliceValue converts dynamic manifest field values into []string. +// It accepts []any and []string inputs; []string inputs are defensively copied. +// It returns (nil, false) when the input is nil, not a string slice, or contains +// non-string elements. func stringSliceValue(value any) ([]string, bool) { switch raw := value.(type) { case []any: @@ -803,6 +807,8 @@ func isRepositoryPackageManifestNotFound(err error) bool { return errors.Is(err, errRepositoryPackageManifestNotFound) } +// isValidManifestVersionFormat validates the aw.yml version-constraint format: +// exact semantic version tags in vMAJOR.minor.patch form. func isValidManifestVersionFormat(version string) bool { const expectedManifestMinVersionDotCount = 2 return semverutil.IsActionVersionTag(version) && strings.Count(strings.TrimPrefix(version, "v"), ".") == expectedManifestMinVersionDotCount