Skip to content

safe-outputs.create-pull-request.draft: false is ignored when agent specifies draft: true #20359

@UncleBats

Description

@UncleBats

Summary

The safe-outputs.create-pull-request.draft configuration setting can be overridden by the AI agent, defeating the purpose of having a configuration guardrail. When draft: false is configured, PRs should NEVER be created as drafts, regardless of what the agent requests.

Analysis

Current behavior (Line 613 in actions/setup/js/create_pull_request.cjs):

const draft = pullRequestItem.draft !== undefined ? pullRequestItem.draft : draftDefault;

This prioritizes the agent's message over the configuration:

  • ✅ If agent specifies draft: true, PR is created as draft (WRONG - bypasses config)
  • ✅ If agent specifies draft: false, PR is not draft
  • ✅ If agent doesn't specify, uses config default

Expected behavior:
Configuration should be enforced as a policy, not used as a fallback. Similar to how autoMerge and allowEmpty work.

Security impact:
This violates the four-layer security architecture documented in scratchpad/dev.md:

Layer 1: Frontmatter Configuration - Workflow authors declare safe-outputs: in YAML frontmatter. Defines operation limits, permissions, and constraints. No runtime modification possible.

Evidence

Configuration shows draft: false

safe-outputs:
  create-pull-request:
    draft: false  # Explicitly configured as false

Log shows draft default set to false, but Draft: true in output

Draft default: false
...
Draft: true  # Agent overrode the configuration!

Agent output specifies "draft": true

{"type": "create_pull_request", "draft": true, ...}

Root Cause

The draft setting is treated as a "fallback" value rather than a policy enforcement:

  1. Agent can include "draft": true/false in its message
  2. If present, agent's value is used (line 613)
  3. Configuration is only consulted if agent doesn't specify

This is inconsistent with other config-only settings:

  • autoMerge - Config only, not overridable ✅
  • allowEmpty - Config only, not overridable ✅
  • draft - Agent can override ❌ (BUG)

Implementation Plan

1. Change Configuration Semantics

The draft configuration should become a policy enforcement mechanism, not a fallback:

Option A: Strict Mode (Recommended)

  • When draft is explicitly configured, use ONLY the config value
  • Ignore any draft field in the agent message
  • Log a warning if agent tries to override

Option B: Config as Default with Warning

  • Keep current fallback behavior
  • Add warning when agent overrides config
  • This maintains backward compatibility but still allows overrides

Recommendation: Use Option A (strict enforcement) because:

  • Aligns with security architecture (Layer 1 configuration)
  • Consistent with autoMerge and allowEmpty behavior
  • Configuration exists specifically to constrain agent behavior

2. Update create_pull_request.cjs

File: actions/setup/js/create_pull_request.cjs

Change at line 613:

// BEFORE (incorrect - agent can override):
const draft = pullRequestItem.draft !== undefined ? pullRequestItem.draft : draftDefault;

// AFTER (correct - config is enforced):
const draft = draftDefault;

// Log warning if agent attempted to override
if (pullRequestItem.draft !== undefined && pullRequestItem.draft !== draftDefault) {
  core.warning(
    `Agent requested draft: ${pullRequestItem.draft}, but configuration enforces draft: ${draftDefault}. ` +
    `Configuration takes precedence for security. To change this, update safe-outputs.create-pull-request.draft in the workflow file.`
  );
}

3. Update Tests

File: actions/setup/js/create_pull_request.test.cjs (if exists) or create new test file

Add test cases:

describe("draft configuration enforcement", () => {
  test("should enforce draft: false from config even when agent requests draft: true", async () => {
    const config = { draft: false };
    const message = { 
      title: "Test PR", 
      body: "Test", 
      draft: true  // Agent tries to override
    };
    
    // Mock GitHub API to capture the draft value
    const createPullMock = jest.fn().mockResolvedValue({ data: { number: 1 } });
    
    // ... test execution ...
    
    // Assert that PR was created with draft: false (from config)
    expect(createPullMock).toHaveBeenCalledWith(
      expect.objectContaining({ draft: false })
    );
  });

  test("should enforce draft: true from config even when agent requests draft: false", async () => {
    const config = { draft: true };
    const message = { 
      title: "Test PR", 
      body: "Test", 
      draft: false  // Agent tries to override
    };
    
    // ... test execution ...
    
    // Assert that PR was created with draft: true (from config)
    expect(createPullMock).toHaveBeenCalledWith(
      expect.objectContaining({ draft: true })
    );
  });

  test("should log warning when agent attempts to override draft config", async () => {
    const config = { draft: false };
    const message = { draft: true };
    
    const warningMock = jest.spyOn(core, 'warning');
    
    // ... test execution ...
    
    expect(warningMock).toHaveBeenCalledWith(
      expect.stringContaining("Agent requested draft: true, but configuration enforces draft: false")
    );
  });
});

4. Update Documentation

File: Documentation files that describe safe-outputs.create-pull-request.draft

Clarify that:

  • draft is a policy setting, not a default
  • Agent messages cannot override this configuration
  • This is consistent with other policy settings like auto-merge and allow-empty

5. Validation Checklist

Before completing:

  • Run make fmt - Format JavaScript files
  • Run make lint-cjs - Validate JavaScript changes
  • Run tests to ensure no regressions
  • Run make agent-finish - Complete validation
  • Verify that draft configuration is now enforced in both directions (false→false, true→true)
  • Confirm warning is logged when agent attempts override

Expected Outcomes

After fix:

  1. draft: false in config → PR is never created as draft
  2. draft: true in config → PR is always created as draft
  3. ✅ Agent cannot bypass this security guardrail
  4. ✅ Warning logged when agent attempts to override
  5. ✅ Consistent with autoMerge and allowEmpty behavior
  6. ✅ Aligns with Layer 1 security architecture

Breaking Changes

Potential impact:

  • Workflows relying on agent overriding draft config will change behavior
  • However, this is a security fix - agent should not be able to bypass configuration guardrails
  • The configuration already exists specifically to constrain agent behavior

Migration:

  • Users who want dynamic draft control should remove draft from their config (then fallback to agent decision)
  • Users who configured draft explicitly were expecting it to be enforced (this fixes that expectation)

Suggested labels: bug, actions, priority-high

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions