feat: safe update mode enforcement for secrets and actions (#safe-update)#24968
feat: safe update mode enforcement for secrets and actions (#safe-update)#24968
Conversation
- Add GHAW manifest (version, secrets, actions) to lock file header - Parse manifest from existing lock files before recompilation - Enforce safe update mode: reject new restricted secrets vs manifest - Allow secrets.GITHUB_TOKEN always; all others must be in manifest - Add safeUpdate field to Compiler struct with CLI override semantics - Add safe-update frontmatter field to FrontmatterConfig - Add --safe-update flag to compile command - Add SafeUpdate field to CompileConfig - Integrate enforcement into CompileWorkflowData flow - Add tests for manifest parsing, enforcement, effectiveSafeUpdate" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9c7b0f2f-b244-44b5-9b0b-d9a9009a9d89 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Change generateYAML and generateAndValidateYAML to return collected body secrets alongside YAML content, eliminating duplicate CollectSecretReferences scan - Update all call sites in test files to use new 3-return-value signature - Fix typo: 'Normalise' -> 'Normalize' in comment - Body secrets improve enforcement accuracy (only workflow body, not header JSON)" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9c7b0f2f-b244-44b5-9b0b-d9a9009a9d89 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Improve GHAW manifest serialization failure log message with impact context - Improve GHAW manifest parse failure log message with behavior description - Simplify buildSafeUpdateError using fmt.Errorf with Join instead of strings.Builder" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9c7b0f2f-b244-44b5-9b0b-d9a9009a9d89 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pkg/workflow/safe_update_manifest.go
Outdated
| var safeUpdateManifestLog = logger.New("workflow:safe_update_manifest") | ||
|
|
||
| // ghawManifestPattern matches a "# GHAW manifest: {...}" line in a lock file header. | ||
| var ghawManifestPattern = regexp.MustCompile(`#\s*GHAW manifest:\s*(\{.+\})`) |
There was a problem hiding this comment.
Done in commit 0f5648f. Renamed all occurrences from # GHAW manifest: to # gh-aw-manifest:, including the regex pattern, log messages, and error strings.
- Rename '# GHAW manifest:' header to '# gh-aw-manifest:' per review - Remove SafeUpdate *bool from FrontmatterConfig; safe update can only be enabled via CLI --safe-update flag (not frontmatter) - Simplify effectiveSafeUpdate to only check compiler CLI flag - Update remediation error message to remove frontmatter reference - Update tests for effectiveSafeUpdate and manifest extraction" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ef674059-8bd8-421d-8740-2382d729353c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit |
|
@copilot adding or remove custom actions that are not approved is also tagged as an error in safe-update mode |
|
@copilot if there is no prior manifest, and no prior lock file, dissallow any secrets usage. |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d8dde9af-177a-43c4-9ad8-bb638163c532 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- EnforceSafeUpdate now accepts actionRefs []string in addition to secretNames - New collectActionViolations function detects added/removed action repos (uses repo as key; SHA/version changes to approved repos are permitted) - buildSafeUpdateError extended to report secret, added-action and removed-action violations in separate clearly-labelled sections - generateYAML now returns (yaml, secrets, actions, error) to propagate action refs without a second scan - generateAndValidateYAML updated to match; compiler_string_api.go updated - All test call sites updated; new test cases added for action enforcement" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d8dde9af-177a-43c4-9ad8-bb638163c532 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit
The error message clearly labels each category: Action refs are now returned alongside secrets from Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…ists When --safe-update is active and there is no existing lock file (nil manifest), treat the manifest as empty rather than skipping enforcement. This ensures that any non-GITHUB_TOKEN secrets and any custom actions are rejected on the very first compilation, matching the principle of least privilege. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a2834c03-20e6-4967-bf03-06e41c3f905f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
…rom enforcement
- Add SafeUpdateFeatureFlag constant ("safe-update") to pkg/constants/feature_constants.go
- Update effectiveSafeUpdate to check isFeatureEnabled(SafeUpdateFeatureFlag) in addition
to the CLI --safe-update flag, allowing safe-update to be enabled via the
frontmatter features section or GH_AW_FEATURES env var
- Exempt actions/* repos (GitHub Actions org) from action violation checks:
adding or removing actions/checkout, actions/setup-node etc. is always allowed
- Update tests to cover feature-flag activation and actions/* exemption
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/295f5415-166c-4ec6-8a31-72b95c074c46
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot read the lock.yml previous version from the previous commit in git/github to avoid local tampering |
Add gitutil.ReadFileFromHEAD() that runs `git show HEAD:<relpath>` to retrieve a file's content from the last committed revision rather than the working tree. In safe update mode, the compiler now uses the committed lock file as the manifest baseline instead of os.ReadFile(). This prevents a local agent from tampering with the .lock.yml file on disk before invoking the compiler in order to forge a manifest that approves new secrets or actions. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ac0875c5-6338-4dee-8b1c-8aa6707ab111 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit Added The compiler now uses this in safe update mode: Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…g in log message Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ac0875c5-6338-4dee-8b1c-8aa6707ab111 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot gate this feature to dev mode only for the compiler. |
In dev mode (local development) the compiler reads the lock file manifest from git HEAD to prevent a local agent from forging an approval by modifying the .lock.yml before invoking the compiler. In release/action modes the compiler runs inside a GitHub Actions runner whose working tree is already a trusted checkout, so the lock file is read directly from the filesystem — avoiding any dependency on git availability in the runner environment. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d5256d85-7fbe-4e3e-b3d9-b7bc6782fe9d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit |
…non-dev mode) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d5256d85-7fbe-4e3e-b3d9-b7bc6782fe9d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a “safe update mode” to the workflow compiler that records an allowlist-style manifest (secrets + external actions) into lock files and, when enabled, rejects compilations that introduce new restricted secret usage or unapproved action repo additions/removals relative to the previously recorded manifest.
Changes:
- Embed a
# gh-aw-manifest: {...}JSON header in generated.lock.ymlfiles and add helpers to parse/serialize it. - Add safe-update enforcement that diffs newly-detected secrets/actions against the previous manifest (with a tamper-resistant baseline in dev mode by reading from
git HEAD). - Wire safe-update enabling via CLI/config and update compiler APIs/tests for new return values from YAML generation.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/compiler_types.go | Adds safeUpdate state + option/setter; (docs need alignment with actual enablement behavior). |
| pkg/gitutil/gitutil.go | Adds ReadFileFromHEAD used to load committed lockfile content as a baseline in dev mode. |
| pkg/gitutil/gitutil_test.go | Adds unit tests for ReadFileFromHEAD. |
| pkg/constants/feature_constants.go | Introduces SafeUpdateFeatureFlag constant. |
| pkg/cli/compile_config.go | Adds SafeUpdate to compile configuration. |
| pkg/cli/compile_compiler_setup.go | Propagates SafeUpdate config into compiler setup and logs enablement. |
| cmd/gh-aw/main.go | Adds --safe-update CLI flag wiring. |
| pkg/workflow/*_test.go (multiple) | Updates tests for the new generateYAML / generateAndValidateYAML return signature. |
| .github/workflows/*.lock.yml (many) | Inserts # gh-aw-manifest: ... header line into generated lock files. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/gitutil/gitutil.go:122
- ReadFileFromHEAD uses cmd.Output() and then wraps any git error as "file not found in HEAD commit", which can mask other failures (e.g., git not available, invalid ref, permissions) and drops stderr. Using CombinedOutput (or capturing stderr) and returning a more accurate error message based on the failure would make debugging much easier.
- Files reviewed: 207/207 changed files
- Comments generated: 6
pkg/workflow/compiler_types.go
Outdated
| // SetSafeUpdate configures whether to enforce safe update mode (reject newly introduced secrets). | ||
| // Safe update mode can only be enabled via the CLI; frontmatter cannot enable it. | ||
| func (c *Compiler) SetSafeUpdate(safeUpdate bool) { | ||
| c.safeUpdate = safeUpdate | ||
| } |
There was a problem hiding this comment.
SetSafeUpdate’s doc comment says safe update mode can only be enabled via the CLI and that frontmatter cannot enable it, but effectiveSafeUpdate() enables it via GH_AW_FEATURES and frontmatter features (features: safe-update: true). Please update this comment (or the behavior) so the API docs match the actual precedence rules.
pkg/workflow/compiler_types.go
Outdated
| // WithSafeUpdate configures whether to enforce safe update mode (reject newly introduced secrets) | ||
| func WithSafeUpdate(safeUpdate bool) CompilerOption { | ||
| return func(c *Compiler) { c.safeUpdate = safeUpdate } | ||
| } |
There was a problem hiding this comment.
The WithSafeUpdate comment mentions only rejecting newly introduced secrets, but safe update enforcement also blocks unapproved action additions/removals. Consider updating the comment to mention actions too (or rename to reflect the broader policy) to avoid misleading callers.
| // git show requires the path to be relative to the repository root and to use | ||
| // forward slashes even on Windows. | ||
| relPath, err := filepath.Rel(gitRoot, absPath) | ||
| if err != nil { | ||
| return "", fmt.Errorf("cannot compute path of %q relative to git root %q: %w", absPath, gitRoot, err) | ||
| } | ||
| relPath = filepath.ToSlash(relPath) |
There was a problem hiding this comment.
ReadFileFromHEAD computes relPath via filepath.Rel(gitRoot, absPath) but doesn’t validate that the resulting path stays within the repo (e.g., relPath starting with ".." when filePath is outside gitRoot). Add a guard to reject paths outside the repository before passing them to git show.
This issue also appears on line 117 of the same file.
cmd/gh-aw/main.go
Outdated
| compileCmd.Flags().Bool("fail-fast", false, "Stop at the first validation error instead of collecting all errors") | ||
| compileCmd.Flags().Bool("no-check-update", false, "Skip checking for gh-aw updates") | ||
| compileCmd.Flags().String("schedule-seed", "", "Override the repository slug (owner/repo) used as seed for fuzzy schedule scattering (e.g. 'github/gh-aw'). Bypasses git remote detection entirely. Use this when your git remote is not named 'origin' and you have multiple remotes configured") | ||
| compileCmd.Flags().Bool("safe-update", false, "Enforce safe update mode: reject compilations that introduce new restricted secrets not present in the existing GHAW manifest. CLI override always takes precedence over frontmatter safe-update field") |
There was a problem hiding this comment.
The --safe-update flag help text says the CLI override always takes precedence over frontmatter safe-update, but the implementation only provides an enabling flag (frontmatter/features can still enable safe update when the CLI flag is not set). Consider rewording to something like “CLI flag forces safe update on (cannot be disabled by frontmatter)” to match behavior.
| Stats bool // Display statistics table sorted by file size | ||
| FailFast bool // Stop at first error instead of collecting all errors | ||
| ScheduleSeed string // Override repository slug used for fuzzy schedule scattering (e.g. owner/repo) | ||
| SafeUpdate bool // Enforce safe update mode: reject compilations that introduce new restricted secrets | ||
| } |
There was a problem hiding this comment.
The SafeUpdate field comment mentions only “restricted secrets”, but safe update mode also enforces an action allowlist policy (added/removed unapproved actions). Update this comment so CLI config accurately reflects what enabling this flag does.
pkg/cli/compile_compiler_setup.go
Outdated
| // Set safe update flag: when enabled via CLI it overrides any frontmatter setting | ||
| compiler.SetSafeUpdate(config.SafeUpdate) | ||
| if config.SafeUpdate { | ||
| compileCompilerSetupLog.Print("Safe update mode enabled: compilations introducing new restricted secrets will be rejected") | ||
| } |
There was a problem hiding this comment.
The log message for enabling safe update mentions only “new restricted secrets”, but enforcement also rejects unapproved action additions/removals. Consider updating the message so users aren’t surprised when action changes are blocked too.
|
@copilot review comments |
…g/help text - compiler_types.go: fix WithSafeUpdate and SetSafeUpdate doc comments to mention action enforcement and accurate enabling paths (CLI/env/frontmatter) - gitutil.go: add path-traversal guard in ReadFileFromHEAD — reject paths whose relative form starts with ".." (outside repository root) - main.go: reword --safe-update flag help text to describe additive enabling semantics and mention action allowlist enforcement - compile_config.go: extend SafeUpdate field comment to mention actions - compile_compiler_setup.go: update log message to mention action enforcement Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ce9fbee6-0a8b-49f6-b289-d275925870c0 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in commit
|
Summary
Introduces safe update mode in the compiler to prevent unauthorized addition of secret consumption and unapproved action changes in workflows. When enabled, the compiler compares previously recorded secrets and actions (from a
gh-aw-manifestheader in the lock file) against newly detected ones and rejects unsafe changes.Changes
New files
pkg/workflow/safe_update_manifest.go—GHAWManifeststruct (version, secrets, actions) with JSON parse/serialize/extract helperspkg/workflow/safe_update_enforcement.go—EnforceSafeUpdatefunction implementing the diff-based secret and action policypkg/workflow/safe_update_manifest_test.go— Tests for manifest parsing, serialization, normalizationpkg/workflow/safe_update_enforcement_test.go— Tests for enforcement logic, action violation detection, andeffectiveSafeUpdateModified files
pkg/constants/feature_constants.go— AddedSafeUpdateFeatureFlag = "safe-update"constantpkg/gitutil/gitutil.go— AddedReadFileFromHEAD(filePath string)to read a file's content from the last git commit (git show HEAD:<relpath>) rather than the working treepkg/workflow/compiler_types.go— AddedsafeUpdate boolfield,WithSafeUpdateoption,SetSafeUpdatemethodpkg/workflow/compiler_yaml.go— UpdatedeffectiveSafeUpdateto check CLI flag and feature flag; embed# gh-aw-manifest: {...}in lock file header;generateYAMLnow returns collected body secrets and action refs (eliminates duplicate scan)pkg/workflow/compiler.go— Reads lock file manifest before recompilation (from git HEAD in dev mode, from filesystem in release/action modes); callsEnforceSafeUpdateafter YAML generation; updatedgenerateAndValidateYAMLto propagate body secrets and action refspkg/workflow/compiler_string_api.go— Updated to handle newgenerateAndValidateYAMLreturn signaturepkg/cli/compile_config.go— AddedSafeUpdate boolfieldpkg/cli/compile_compiler_setup.go— PassSafeUpdatefrom config to compilercmd/gh-aw/main.go— Added--safe-updateflag; wire toCompileConfigBehavior
Safe update mode can be enabled via the
--safe-updateCLI flag, theGH_AW_FEATURES=safe-updateenvironment variable, or the workflow frontmatterfeatures: safe-update: truesection.actions/*actions rejected--safe-updateflag or feature flag)--safe-updateCLI flag orfeatures: safe-update: truesecrets.GITHUB_TOKENactions/*actions (GitHub Actions org)actions/*repo not in manifest)actions/*action removedTamper Prevention
The manifest baseline source depends on the compiler's action mode:
git show HEAD:<lockfile>) rather than the working tree. This prevents a local agent from modifying the.lock.ymlon disk before invoking the compiler to forge an approved manifest and bypass enforcement. If the lock file has never been committed, enforcement proceeds with an empty manifest (principle of least privilege).Manifest format
The manifest line is embedded immediately after
# gh-aw-metadata:in generated lock files and updated on every successful compilation.Error output
When violations are detected, the error clearly labels each category: