From 0459433c1e49721f4a84d67b89bd5bcbe9ddc1a5 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 15 Aug 2025 16:32:29 +0100 Subject: [PATCH 1/4] add more security notes --- docs/security-notes.md | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/security-notes.md b/docs/security-notes.md index 7f727a632b..48023b2ed5 100644 --- a/docs/security-notes.md +++ b/docs/security-notes.md @@ -49,6 +49,8 @@ In addition, the compilation step of Agentic Workflows enforces additional secur - **Tool allowlisting** - only explicitly allowed tools can be used in the workflow - **Highly restricted commands** - by default, no commands are allowed to be executed, and any commands that are allowed must be explicitly specified in the workflow - **Explicit tool allowlisting** - only tools explicitly allowed in the workflow can be used +- **Limit workflow longevity** - workflows can be configured to stop running after a certain time period, preventing long-running or runaway workflows +- **Limit chat iterations** - workflows can be configured to limit the number of chat iterations per run, preventing runaway loops and excessive resource consumption Apply these principles consistently across all workflow components: @@ -87,7 +89,9 @@ GitHub Actions workflows are designed to be steps within a larger process. Some - **Plan-apply separation**: Implement a "plan" phase that generates a preview of actions before execution. This allows human reviewers to assess the impact of changes. This is usually done via an output issue or pull request. - **Review and audit**: Regularly review workflow history, permissions, and tool usage to ensure compliance with security policies. -### Limit time of operation +### Limit operations + +#### Limit workflow longevity by `stop-time:` Use `stop-time:` to limit the time of operation of an agentic workflow. For example, using @@ -97,6 +101,20 @@ stop-time: +7d will mean the agentic workflow no longer operates 7 days after time of compilation. +#### Limit workflow runs by `max-turns:` + +Use `max-turns:` to limit the number of chat iterations per run. This prevents runaway loops and excessive resource consumption. For example: + +```yaml +max-turns: 5 +``` + +This limits the workflow to a maximum of 5 interactions with the AI engine per run. + +#### Monitor costs by `gh aw logs` + +Use `gh aw logs` to monitor the costs of running agentic workflows. This command provides insights into the number of turns, tokens used, and other metrics that can help you understand the cost implications of your workflows. Reported information may differ based on the AI engine used (e.g., Claude vs. Codex). + ### MCP Tool Hardening Model Context Protocol tools require strict containment: From 524027954b8054fb3965f3b0d47acb8f5b7071f8 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 15 Aug 2025 16:40:34 +0100 Subject: [PATCH 2/4] fix default permissions --- pkg/workflow/compiler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 9a6e484ada..1206a8fd6f 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -971,7 +971,7 @@ func (c *Compiler) applyDefaults(data *WorkflowData, markdownPath string) { data.Permissions = `permissions: contents: read issues: read - pull_requests: read + pull-requests: read discussions: read deployments: read models: read` From be70e06a409465acbbcb4a36af4ec89493c656aa Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 15 Aug 2025 16:45:22 +0100 Subject: [PATCH 3/4] udpate --- pkg/workflow/compiler_test.go | 79 ++++++++++++++++++++++++++++++----- 1 file changed, 69 insertions(+), 10 deletions(-) diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index b1d188c5ce..3fb9adcb06 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -4578,12 +4578,14 @@ This workflow should get default permissions applied automatically. compiler := NewCompiler(false, "", "test") // Compile the workflow - lockFile := filepath.Join(tmpDir, "test-default-permissions.lock.yml") - err = compiler.CompileWorkflow(testFile, lockFile) + err = compiler.CompileWorkflow(testFile) if err != nil { t.Fatalf("Failed to compile workflow: %v", err) } + // Calculate the lock file path + lockFile := strings.TrimSuffix(testFile, ".md") + ".lock.yml" + // Read the generated lock file lockContent, err := os.ReadFile(lockFile) if err != nil { @@ -4592,6 +4594,9 @@ This workflow should get default permissions applied automatically. lockContentStr := string(lockContent) + // Debug: Print the generated content for inspection + t.Logf("Generated workflow content:\n%s", lockContentStr) + // Verify that default permissions are present in the generated workflow expectedDefaultPermissions := []string{ "contents: read", @@ -4619,10 +4624,36 @@ This workflow should get default permissions applied automatically. t.Fatalf("Failed to parse generated YAML: %v", err) } - // Verify permissions section exists in parsed YAML - permissions, exists := workflow["permissions"] + // Verify that jobs section exists + jobs, exists := workflow["jobs"] + if !exists { + t.Fatal("Jobs section not found in parsed workflow") + } + + jobsMap, ok := jobs.(map[string]interface{}) + if !ok { + t.Fatal("Jobs section is not a map") + } + + // Find the main job (should be the one with the workflow name converted to kebab-case) + var mainJob map[string]interface{} + for jobName, job := range jobsMap { + if jobName == "test-workflow" { // The workflow name "Test Workflow" becomes "test-workflow" + if jobMap, ok := job.(map[string]interface{}); ok { + mainJob = jobMap + break + } + } + } + + if mainJob == nil { + t.Fatal("Main workflow job not found") + } + + // Verify permissions section exists in the main job + permissions, exists := mainJob["permissions"] if !exists { - t.Fatal("Permissions section not found in parsed workflow") + t.Fatal("Permissions section not found in main job") } // Verify permissions is a map @@ -4688,12 +4719,14 @@ This workflow has custom permissions that should override defaults. compiler := NewCompiler(false, "", "test") // Compile the workflow - lockFile := filepath.Join(tmpDir, "test-custom-permissions.lock.yml") - err = compiler.CompileWorkflow(testFile, lockFile) + err = compiler.CompileWorkflow(testFile) if err != nil { t.Fatalf("Failed to compile workflow: %v", err) } + // Calculate the lock file path + lockFile := strings.TrimSuffix(testFile, ".md") + ".lock.yml" + // Read the generated lock file lockContent, err := os.ReadFile(lockFile) if err != nil { @@ -4706,10 +4739,36 @@ This workflow has custom permissions that should override defaults. t.Fatalf("Failed to parse generated YAML: %v", err) } - // Verify permissions section exists in parsed YAML - permissions, exists := workflow["permissions"] + // Verify that jobs section exists + jobs, exists := workflow["jobs"] + if !exists { + t.Fatal("Jobs section not found in parsed workflow") + } + + jobsMap, ok := jobs.(map[string]interface{}) + if !ok { + t.Fatal("Jobs section is not a map") + } + + // Find the main job (should be the one with the workflow name converted to kebab-case) + var mainJob map[string]interface{} + for jobName, job := range jobsMap { + if jobName == "test-workflow" { // The workflow name "Test Workflow" becomes "test-workflow" + if jobMap, ok := job.(map[string]interface{}); ok { + mainJob = jobMap + break + } + } + } + + if mainJob == nil { + t.Fatal("Main workflow job not found") + } + + // Verify permissions section exists in the main job + permissions, exists := mainJob["permissions"] if !exists { - t.Fatal("Permissions section not found in parsed workflow") + t.Fatal("Permissions section not found in main job") } // Verify permissions is a map From 834075def245d29b6ac2714bae62448b8fe60947 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 15 Aug 2025 16:48:18 +0100 Subject: [PATCH 4/4] udpate --- pkg/workflow/compiler_test.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/pkg/workflow/compiler_test.go b/pkg/workflow/compiler_test.go index 3fb9adcb06..8404bf0044 100644 --- a/pkg/workflow/compiler_test.go +++ b/pkg/workflow/compiler_test.go @@ -4594,13 +4594,10 @@ This workflow should get default permissions applied automatically. lockContentStr := string(lockContent) - // Debug: Print the generated content for inspection - t.Logf("Generated workflow content:\n%s", lockContentStr) - // Verify that default permissions are present in the generated workflow expectedDefaultPermissions := []string{ "contents: read", - "issues: read", + "issues: read", "pull-requests: read", "discussions: read", "deployments: read", @@ -4664,12 +4661,12 @@ This workflow should get default permissions applied automatically. // Verify each expected default permission exists and has correct value expectedPermissionsMap := map[string]string{ - "contents": "read", - "issues": "read", - "pull-requests": "read", - "discussions": "read", - "deployments": "read", - "models": "read", + "contents": "read", + "issues": "read", + "pull-requests": "read", + "discussions": "read", + "deployments": "read", + "models": "read", } for key, expectedValue := range expectedPermissionsMap { @@ -4799,7 +4796,7 @@ This workflow has custom permissions that should override defaults. lockContentStr := string(lockContent) defaultOnlyPermissions := []string{ "pull-requests: read", - "discussions: read", + "discussions: read", "deployments: read", "models: read", }