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

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions