Skip to content

Add compilation warning for id-token: write permission #15026

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 3 commits into from
Feb 11, 2026
Merged

Add compilation warning for id-token: write permission #15026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
905093d
Initial plan
Copilot Feb 11, 2026
6eda57f
Add compilation warning for id-token: write permission
Copilot Feb 11, 2026
9231dd4
Merge branch 'main' into copilot/add-compilation-warning-id-token
pelikhan Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions pkg/workflow/compiler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,22 @@ func (c *Compiler) validateWorkflowData(workflowData *WorkflowData, markdownPath
}
}

// Emit warning if id-token: write permission is detected
log.Printf("Checking for id-token: write permission")
if workflowData.Permissions != "" {
Comment on lines +258 to +260
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log.Printf("Checking for id-token: write permission") will be written to stderr by the standard logger, and it contains the exact substring id-token: write. That makes stderr-based warning detection (including the new tests) flaky and also pollutes compiler output. Please remove this log line or gate it behind an explicit verbose/debug flag and avoid emitting id-token: write verbatim in logs.

See below for a potential fix:

						fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", message))
						c.IncrementWarningCount()
					}
				}
			}
		}
	}

	// Emit warning if id-token: write permission is detected

Copilot uses AI. Check for mistakes.
permissions := NewPermissionsParser(workflowData.Permissions).ToPermissions()
if permissions != nil {
Comment on lines +260 to +262
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewPermissionsParser(workflowData.Permissions).ToPermissions() is invoked again here even though permissions were already parsed earlier in this function for MCP toolset validation. Consider parsing once and reusing the result to avoid duplicated logic and keep warning/validation behavior consistent.

Copilot uses AI. Check for mistakes.
level, exists := permissions.Get(PermissionIdToken)
if exists && level == PermissionWrite {
warningMsg := `This workflow grants id-token: write permission
OIDC tokens can authenticate to cloud providers (AWS, Azure, GCP).
Ensure proper audience validation and trust policies are configured.`
Comment on lines +265 to +267
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The raw string literal for warningMsg includes leading spaces on the 2nd/3rd lines due to code indentation, which will render as extra indentation in the warning output and won’t match the example output in the PR description. Consider left-aligning the message text (or trimming indentation) so the emitted warning is clean and consistent.

Suggested change
warningMsg := `This workflow grants id-token: write permission
OIDC tokens can authenticate to cloud providers (AWS, Azure, GCP).
Ensure proper audience validation and trust policies are configured.`
warningMsg := "This workflow grants id-token: write permission\n" +
"OIDC tokens can authenticate to cloud providers (AWS, Azure, GCP).\n" +
"Ensure proper audience validation and trust policies are configured."

Copilot uses AI. Check for mistakes.
fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", warningMsg))
c.IncrementWarningCount()
}
}
}

// Validate GitHub tools against enabled toolsets
log.Printf("Validating GitHub tools against enabled toolsets")
if workflowData.ParsedTools != nil && workflowData.ParsedTools.GitHub != nil {
Expand Down
228 changes: 228 additions & 0 deletions pkg/workflow/idtoken_write_warning_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
//go:build integration

package workflow

import (
"bytes"
"io"
"os"
"path/filepath"
"strings"
"testing"

"github.com/github/gh-aw/pkg/testutil"
)

// TestIdTokenWriteWarning tests that id-token: write permission emits a warning
func TestIdTokenWriteWarning(t *testing.T) {
tests := []struct {
name string
content string
expectWarning bool
}{
{
name: "id-token write produces warning",
content: `---
on: workflow_dispatch
engine: copilot
permissions:
contents: read
id-token: write
---

# Test Workflow
`,
expectWarning: true,
},
{
name: "id-token read does not produce warning",
content: `---
on: workflow_dispatch
engine: copilot
permissions:
contents: read
id-token: read
---

# Test Workflow
`,
expectWarning: false,
},
{
name: "no id-token does not produce warning",
content: `---
on: workflow_dispatch
engine: copilot
permissions:
contents: read
issues: read
---

# Test Workflow
`,
expectWarning: false,
},
{
name: "id-token write with other permissions produces warning",
content: `---
on: workflow_dispatch
engine: copilot
permissions:
contents: read
issues: read
pull-requests: read
id-token: write
---

# Test Workflow
`,
expectWarning: true,
},
{
name: "id-token write only produces warning",
content: `---
on: workflow_dispatch
engine: copilot
permissions:
id-token: write
---

# Test Workflow
`,
expectWarning: true,
},
{
name: "no permissions does not produce warning",
content: `---
on: workflow_dispatch
engine: copilot
---

# Test Workflow
`,
expectWarning: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tmpDir := testutil.TempDir(t, "idtoken-warning-test")

testFile := filepath.Join(tmpDir, "test-workflow.md")
if err := os.WriteFile(testFile, []byte(tt.content), 0644); err != nil {
t.Fatal(err)
}

// Capture stderr to check for warnings
oldStderr := os.Stderr
r, w, _ := os.Pipe()
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.Pipe() errors are ignored here (r, w, _ := os.Pipe()), which can lead to nil pipes and misleading test failures. Handle the returned error and fail the test if pipe creation fails.

This issue also appears on line 198 of the same file.

Suggested change
r, w, _ := os.Pipe()
r, w, pipeErr := os.Pipe()
if pipeErr != nil {
t.Fatalf("failed to create stderr pipe: %v", pipeErr)
}

Copilot uses AI. Check for mistakes.
os.Stderr = w

compiler := NewCompiler()
compiler.SetStrictMode(false)
err := compiler.CompileWorkflow(testFile)

// Restore stderr
w.Close()
os.Stderr = oldStderr
var buf bytes.Buffer
io.Copy(&buf, r)
Comment on lines +118 to +129
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The read end of the pipe (r) is never closed and io.Copy errors are ignored. Close r (typically with defer r.Close() after successful os.Pipe()) and check the io.Copy return error to avoid FD leaks and silent read failures in CI.

This issue also appears on line 208 of the same file.

Suggested change
r, w, _ := os.Pipe()
os.Stderr = w
compiler := NewCompiler()
compiler.SetStrictMode(false)
err := compiler.CompileWorkflow(testFile)
// Restore stderr
w.Close()
os.Stderr = oldStderr
var buf bytes.Buffer
io.Copy(&buf, r)
r, w, err := os.Pipe()
if err != nil {
t.Fatalf("failed to create stderr pipe: %v", err)
}
defer r.Close()
os.Stderr = w
compiler := NewCompiler()
compiler.SetStrictMode(false)
err = compiler.CompileWorkflow(testFile)
// Restore stderr
w.Close()
os.Stderr = oldStderr
var buf bytes.Buffer
if _, copyErr := io.Copy(&buf, r); copyErr != nil {
t.Fatalf("failed to read from stderr pipe: %v", copyErr)
}

Copilot uses AI. Check for mistakes.
stderrOutput := buf.String()

if err != nil {
t.Errorf("Expected compilation to succeed but it failed: %v", err)
return
}

expectedPhrases := []string{
"id-token: write",
"OIDC tokens can authenticate to cloud providers",
"AWS, Azure, GCP",
"audience validation",
"trust policies",
}

if tt.expectWarning {
for _, phrase := range expectedPhrases {
if !strings.Contains(stderrOutput, phrase) {
t.Errorf("Expected warning to contain '%s', got stderr:\n%s", phrase, stderrOutput)
}
}
// Check for warning indicator
if !strings.Contains(stderrOutput, "warning:") {
t.Errorf("Expected 'warning:' in stderr output, got:\n%s", stderrOutput)
}
} else {
// Should not contain any of the id-token specific warning phrases
for _, phrase := range expectedPhrases {
if strings.Contains(stderrOutput, phrase) {
t.Errorf("Did not expect warning containing '%s', but got stderr:\n%s", phrase, stderrOutput)
}
}
}

// Verify warning count
if tt.expectWarning {
warningCount := compiler.GetWarningCount()
if warningCount == 0 {
t.Error("Expected warning count > 0 but got 0")
}
}
})
}
}

// TestIdTokenWriteWarningMessageFormat tests that the warning message format
// matches the specified format
func TestIdTokenWriteWarningMessageFormat(t *testing.T) {
tmpDir := testutil.TempDir(t, "idtoken-warning-format-test")

content := `---
on: workflow_dispatch
engine: copilot
permissions:
contents: read
id-token: write
---

# Test Workflow
`

testFile := filepath.Join(tmpDir, "test-workflow.md")
if err := os.WriteFile(testFile, []byte(content), 0644); err != nil {
t.Fatal(err)
}

// Capture stderr to check for warnings
oldStderr := os.Stderr
r, w, _ := os.Pipe()
os.Stderr = w

compiler := NewCompiler()
compiler.SetStrictMode(false)
err := compiler.CompileWorkflow(testFile)

// Restore stderr
w.Close()
os.Stderr = oldStderr
var buf bytes.Buffer
io.Copy(&buf, r)
stderrOutput := buf.String()

if err != nil {
t.Fatalf("Expected compilation to succeed but it failed: %v", err)
}

// Verify the exact warning format
expectedLines := []string{
"This workflow grants id-token: write permission",
"OIDC tokens can authenticate to cloud providers (AWS, Azure, GCP).",
"Ensure proper audience validation and trust policies are configured.",
}

for _, line := range expectedLines {
if !strings.Contains(stderrOutput, line) {
t.Errorf("Expected warning to contain line '%s', got stderr:\n%s", line, stderrOutput)
}
}
}
Loading