Enforce manifest-level rejection of workflows that declare private: true

docs/adr/36227-reject-private-field-in-manifest-workflows.md

@@ -0,0 +1,38 @@
+# ADR-36227: Enforce `private: true` for Add/Package Blocking
+
+**Date**: 2026-06-01
+**Status**: Draft
+
+## Context
+
+`aw.yml` package manifests can list installable workflows that other repositories add via `gh aw add` / `gh aw add-wizard`. The `private` frontmatter field is intended to block installation only when it is explicitly set to `true`. Manifest-backed resolution and compile-time validation must preserve that semantics consistently, so package workflows only fail when they declare `private: true`.
+
+## Decision
+
+We keep `ExtractWorkflowPrivateSetting(content) (value, present bool)` for frontmatter parsing, but manifest-backed resolution (`ResolveWorkflows` for `FromRepositoryManifest` specs) and compile-time local manifest validation reject only `private: true`. `private: false` remains installable.
+
+## Alternatives Considered
+
+### Alternative 1: Reject any manifest-listed `private` declaration
+Reject `private: true` and `private: false` equally for manifest-listed workflows. This was rejected because `private: false` is not a disable signal and should not block install/package behavior.
+
+### Alternative 2: Strip or normalize the `private` field during manifest install instead of respecting value
+Silently normalize `private` during manifest resolution. This was rejected because it hides intent; explicit `private: true` should continue to fail loudly.
+
+## Consequences
+
+### Positive
+- Add/package and compile-time validation consistently reject only workflows that declare `private: true`.
+- Error messages continue to name the offending workflow path when rejection occurs.
+
+### Negative
+- Existing manifests using `private: false` remain installable; only `private: true` workflows are blocked.
+- Introduces a second extraction function (`ExtractWorkflowPrivateSetting` alongside `ExtractWorkflowPrivate`) and parallel checks in both the resolution and compile-time paths that must stay in sync.
+
+### Neutral
+- Standalone (non-manifest) workflow adds are unchanged: `private: true` still blocks installation via the preserved `ExtractWorkflowPrivate` path.
+- Compile-time validation scans manifest-listed installable paths (explicit `files:`/`includes:` entries, or discovered package directories) using the same `private: true` semantics.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/26752819392) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/cli/add_package_manifest.go

@@ -705,6 +705,26 @@ func normalizePackageInstallablePaths(paths []string, packagePath string) []stri
 	return normalized
 }
 
+func validateManifestInstallableWorkflowPrivacy(manifestPath string, installationSources []string, readWorkflow func(string) ([]byte, error)) error {
+	for _, installationSource := range installationSources {
+		if isActionWorkflowPath(installationSource) {
+			continue
+		}
+
+		content, err := readWorkflow(installationSource)
+		if err != nil {
+			return fmt.Errorf("invalid Agentic Workflow manifest %q: %w", manifestPath, err)
+		}
+
+		privateValue, hasPrivate := ExtractWorkflowPrivateSetting(string(content))
+		if hasPrivate && privateValue {
+			return fmt.Errorf("invalid Agentic Workflow manifest %q: workflow %q sets private: true and cannot be included because private workflows cannot be added", manifestPath, installationSource)
+		}
+	}
+
+	return nil
+}
+
 func isSupportedPackageInstallablePath(p string) bool {
 	// Normalize separators to forward slashes (consistent with joinRepositoryPackagePath) then
 	// clean to reject path traversal (e.g. "workflows/../README.md" → "README.md").

pkg/cli/add_package_manifest_test.go

@@ -473,6 +473,60 @@ files:
 	assert.Equal(t, ".github/workflows/nightly-review.md", resolved.Workflows[1].Spec.WorkflowPath)
 }
 
+func TestResolveWorkflows_RepositoryPackageRejectsPrivateTrue(t *testing.T) {
+	originalFetchFn := fetchWorkflowFromSourceWithContextFn
+	originalDownload := downloadPackageFileFromGitHubForHost
+	originalList := listPackageWorkflowFilesForHost
+	originalDirFiles := listPackageDirFilesForHost
+	originalDirSubdirs := listPackageDirSubdirsForHost
+	originalDefaultBranch := getRepositoryPackageDefaultBranch
+	t.Cleanup(func() {
+		fetchWorkflowFromSourceWithContextFn = originalFetchFn
+		downloadPackageFileFromGitHubForHost = originalDownload
+		listPackageWorkflowFilesForHost = originalList
+		listPackageDirFilesForHost = originalDirFiles
+		listPackageDirSubdirsForHost = originalDirSubdirs
+		getRepositoryPackageDefaultBranch = originalDefaultBranch
+	})
+	getRepositoryPackageDefaultBranch = func(repoSlug, host string) (string, error) {
+		return "main", nil
+	}
+	listPackageDirFilesForHost = func(owner, repo, ref, dirPath, host string) ([]string, error) {
+		return nil, createRepositoryPackageNotFoundError(dirPath)
+	}
+	listPackageDirSubdirsForHost = func(owner, repo, ref, dirPath, host string) ([]string, error) {
+		return nil, createRepositoryPackageNotFoundError(dirPath)
+	}
+	downloadPackageFileFromGitHubForHost = func(owner, repo, path, ref, host string) ([]byte, error) {
+		switch path {
+		case "aw.yml":
+			return []byte("name: Repo Assist\nfiles:\n  - workflows/review.md\n"), nil
+		case "README.md":
+			return []byte("# Repo Assist\n"), nil
+		case "workflows/review.md":
+			return []byte("---\nprivate: true\n---\n\n# Review\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
+	}
+	fetchWorkflowFromSourceWithContextFn = func(_ context.Context, spec *WorkflowSpec, _ bool) (*FetchedWorkflow, error) {
+		return &FetchedWorkflow{
+			Content:    []byte("---\nprivate: true\n---\n\n# Review\n"),
+			CommitSHA:  "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+			IsLocal:    false,
+			SourcePath: spec.WorkflowPath,
+		}, nil
+	}
+
+	_, err := ResolveWorkflows(context.Background(), []string{"owner/repo"}, false)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), `workflow "workflows/review.md" sets private: true`)
+}
+
 func TestResolveWorkflows_NestedRepositoryPackage(t *testing.T) {
 	originalFetchFn := fetchWorkflowFromSourceWithContextFn
 	originalDownload := downloadPackageFileFromGitHubForHost

pkg/cli/add_workflow_resolution.go

@@ -214,6 +214,14 @@ func ResolveWorkflows(ctx context.Context, workflows []string, verbose bool) (*R
 		// Extract engine from content (if specified in frontmatter)
 		engine := ExtractWorkflowEngine(string(fetched.Content))
 
+		if spec.FromRepositoryManifest {
+			privateValue, hasPrivate := ExtractWorkflowPrivateSetting(string(fetched.Content))
+			if hasPrivate && privateValue {
+				manifestPath := joinRepositoryPackagePath(spec.PackagePath, repositoryPackageManifestFileName)
+				return nil, fmt.Errorf("invalid Agentic Workflow manifest %q: workflow %q sets private: true and cannot be included because private workflows cannot be added", manifestPath, resolvedSpec.WorkflowPath)
+			}
+		}
+
 		// Check if workflow is private - private workflows cannot be added to other repositories
 		isPrivate := ExtractWorkflowPrivate(string(fetched.Content))
 		if isPrivate {

pkg/cli/compile_repository_manifest.go

@@ -3,6 +3,7 @@ package cli
 import (
 	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 
@@ -102,10 +103,75 @@ func findLocalRepositoryPackageManifest(gitRoot string) (string, error) {
 func validateLocalRepositoryPackageContents(manifestPath string) error {
 	readmePath := filepath.Join(filepath.Dir(manifestPath), "README.md")
 	if _, err := os.Stat(readmePath); err == nil {
-		return nil
+		manifestContent, err := os.ReadFile(manifestPath)
+		if err != nil {
+			return fmt.Errorf("failed to read Agentic Workflow manifest %q: %w", manifestPath, err)
+		}
+		manifest, _, err := parseRepositoryPackageManifest(manifestPath, manifestContent)
+		if err != nil {
+			return err
+		}
+
+		includeInstallablePaths, _, _ := splitManifestIncludePaths(manifest.Includes)
+		includeInstallablePaths = append(includeInstallablePaths, manifest.Files...)
+		installationSources := normalizePackageInstallablePaths(includeInstallablePaths, "")
+		if len(installationSources) == 0 {
+			installationSources, err = scanLocalRepositoryPackageInstallablePaths(filepath.Dir(manifestPath))
+			if err != nil {
+				return err
+			}
+		}
+
+		return validateManifestInstallableWorkflowPrivacy(manifestPath, installationSources, func(sourcePath string) ([]byte, error) {
+			content, err := os.ReadFile(filepath.Join(filepath.Dir(manifestPath), filepath.FromSlash(sourcePath)))
+			if err != nil {
+				return nil, fmt.Errorf("failed to read workflow %q: %w", sourcePath, err)
+			}
+			return content, nil
+		})
 	} else if os.IsNotExist(err) {
 		return fmt.Errorf("invalid Agentic Workflow manifest %q: missing required README.md", manifestPath)
 	} else {
 		return fmt.Errorf("failed to read package README %q: %w", readmePath, err)
 	}
 }
+
+func scanLocalRepositoryPackageInstallablePaths(packageDir string) ([]string, error) {
+	var collected []string
+	seen := make(map[string]struct{})
+
+	for _, sourceDir := range packageSourceDirectories {
+		sourcePath := filepath.Join(packageDir, filepath.FromSlash(sourceDir))
+		err := filepath.WalkDir(sourcePath, func(currentPath string, d fs.DirEntry, walkErr error) error {
+			if walkErr != nil {
+				return walkErr
+			}
+			if d.IsDir() {
+				return nil
+			}
+
+			relativePath, err := filepath.Rel(packageDir, currentPath)
+			if err != nil {
+				return err
+			}
+			relativePath = filepath.ToSlash(relativePath)
+			if !isSupportedPackageInstallablePath(relativePath) {
+				return nil
+			}
+			if _, exists := seen[relativePath]; exists {
+				return nil
+			}
+			seen[relativePath] = struct{}{}
+			collected = append(collected, relativePath)
+			return nil
+		})
+		if err != nil {
+			if os.IsNotExist(err) {
+				continue
+			}
+			return nil, fmt.Errorf("failed to scan %q: %w", sourcePath, err)
+		}
+	}
+
+	return collected, nil
+}

pkg/cli/compile_repository_manifest_test.go

@@ -180,6 +180,36 @@ name: Repo Assist
 	assert.Contains(t, err.Error(), "missing required README.md")
 }
 
+func TestCompileWorkflows_RejectsManifestWorkflowWithPrivateTrue(t *testing.T) {
+	tmpDir := testutil.TempDir(t, "aw-manifest-private-true-*")
+	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, "workflows"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "workflows", "review.md"), []byte(`---
+private: true
+---
+
+# Review
+`), 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"
+name: Repo Assist
+files:
+  - workflows/review.md
+`), 0o644))
+
+	_, err = CompileWorkflows(context.Background(), CompileConfig{})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), `workflow "workflows/review.md" sets private: true`)
+}
+
 func TestValidateRepositoryManifestForCompilation_PropagatesGitRootErrors(t *testing.T) {
 	originalFindGitRoot := findGitRootForManifestValidation
 	t.Cleanup(func() {

pkg/cli/packages.go

@@ -230,20 +230,30 @@ func ExtractWorkflowEngine(content string) string {
 	return ""
 }
 
-// ExtractWorkflowPrivate extracts the private field from workflow content string.
-// Returns true if the workflow has private: true in its frontmatter.
-func ExtractWorkflowPrivate(content string) bool {
+// ExtractWorkflowPrivateSetting extracts the private field from workflow content string.
+// Returns the boolean value and whether the field was explicitly present.
+func ExtractWorkflowPrivateSetting(content string) (bool, bool) {
 	result, err := parser.ExtractFrontmatterFromContent(content)
 	if err != nil {
-		return false
+		return false, false
 	}
 
 	if private, ok := result.Frontmatter["private"]; ok {
 		if privateBool, ok := private.(bool); ok {
-			return privateBool
+			return privateBool, true
 		}
 	}
 
+	return false, false
+}
+
+// ExtractWorkflowPrivate extracts the private field from workflow content string.
+// Returns true if the workflow has private: true in its frontmatter.
+func ExtractWorkflowPrivate(content string) bool {
+	privateBool, ok := ExtractWorkflowPrivateSetting(content)
+	if ok {
+		return privateBool
+	}
 	return false
 }