Skip to content

Safe-outputs PR constraints not enforced at tool-call time #23412

@lpcox

Description

@lpcox

Problem

Smoke testing of safe-outputs PR operations (see gh-aw-mcpg#2746) reveals that negative enforcement is not applied at tool-call time. All safe-outputs MCP tool calls return {"result":"success"} regardless of whether the operation violates configured constraints (max, prefix, required-labels, reviewers allowlist, body: false).

Test Results Summary

The following constraints were not enforced at the tool-call API level:

Safe-Output Constraint Expected Actual
create-pull-request prefix — call without required prefix ❌ Rejected result:success
create-pull-request max: 1 — second creation attempt ❌ Rejected result:success
update-pull-request body: false — body update when disallowed ❌ Rejected result:success
update-pull-request max: 1 — second update attempt ❌ Rejected result:success
mark-ready required-labels: [smoke-test] — PR without label ❌ Rejected result:success
mark-ready max: 1 — second mark-ready attempt ❌ Rejected result:success
add-reviewer reviewers: [copilot] — non-allowed reviewer ❌ Rejected result:success
add-reviewer max: 1 — second add-reviewer attempt ❌ Rejected result:success
close-pull-request required-labels — PR without required label ❌ Rejected result:success
close-pull-request max: 1 — second close attempt ❌ Rejected result:success

Positive test cases (valid calls) all passed correctly.

Architecture Context

Safe-outputs uses a three-layer architecture:

  1. Compiler (Go, pkg/workflow/) — parses frontmatter constraints → encodes into GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG env var in the compiled .lock.yml
  2. Gateway (JS, actions/setup/js/safe_outputs_handlers.cjs) — loads constraints at runtime, creates handler functions for each enabled tool
  3. Ingestion (JS, post-processing) — applies accumulated safe outputs to GitHub API after the agent completes

The problem is that Layer 2 (gateway handlers) records tool calls to outputs.jsonl without validating constraints, deferring all enforcement to Layer 3 (ingestion). This means:

  • The AI agent never receives error feedback for invalid operations
  • The agent may waste tokens retrying operations it thinks succeeded
  • Constraint violations are only discovered silently during post-processing
  • From the agent's perspective, all operations appear to succeed

Root Cause Analysis

In safe_outputs_handlers.cjs, handler functions like createPullRequestHandler primarily do:

const entry = { ...args, type: "create_pull_request" };
appendSafeOutput(entry);  // Always appends, no constraint checks

Some constraints are validated (e.g., limit_enforcement_helpers.cjs for array limits), but the core constraints (max call count, prefix, required-labels, reviewers allowlist, body: false) are not checked before appending.

Proposed Solution

Phase 1: Tool-Call-Time Rejection (Handler Layer)

Add constraint validation in safe_outputs_handlers.cjs before appendSafeOutput():

1. max enforcement — count-based rejection

Track per-tool call counts and reject once max is exceeded:

const callCounts = {};  // Per-handler call counter

function enforceMax(handlerName, config) {
    const max = parseInt(config.max, 10);
    if (isNaN(max)) return;  // No max configured
    callCounts[handlerName] = (callCounts[handlerName] || 0) + 1;
    if (callCounts[handlerName] > max) {
        throw new Error(`E_MAX_EXCEEDED: ${handlerName} max of ${max} reached`);
    }
}

2. prefix / required-title-prefix enforcement

Validate title against required prefix before accepting the call:

function enforceTitlePrefix(title, prefix) {
    if (prefix && !title.startsWith(prefix)) {
        throw new Error(`E_PREFIX: title must start with "${prefix}"`);
    }
}

3. required-labels enforcement

Query the target PR for labels and reject if missing:

function enforceRequiredLabels(prNumber, requiredLabels) {
    // Fetch PR labels via GitHub API
    // Reject if any required label is missing
}

4. reviewers allowlist enforcement

Reject reviewer additions not in the configured allowlist:

function enforceReviewerAllowlist(requestedReviewers, allowed) {
    const disallowed = requestedReviewers.filter(r => !allowed.includes(r));
    if (disallowed.length > 0) {
        throw new Error(`E_REVIEWER: reviewers not allowed: ${disallowed.join(', ')}`);
    }
}

5. body: false enforcement

Reject body field when update-pull-request has body: false:

function enforceFieldRestrictions(args, config) {
    if (config.body === false && args.body) {
        throw new Error(`E_FIELD: body updates are not allowed`);
    }
}

Phase 2: Error Response Contract

Define a consistent error response for rejected calls so agents can react:

{
    "result": "error",
    "error_code": "E_MAX_EXCEEDED",
    "message": "create_pull_request max of 1 reached",
    "constraint": "max",
    "configured_value": 1
}

Phase 3: Ingestion Double-Check

Keep the existing ingestion-layer enforcement as a defence-in-depth backstop. Even if a constraint bypass occurs at the handler layer, ingestion should still reject non-conforming operations.

Files to Modify

File Change
actions/setup/js/safe_outputs_handlers.cjs Add pre-append constraint validation for all PR handlers
actions/setup/js/safe_outputs_bootstrap.cjs Wire call-count tracking into handler creation
actions/setup/js/limit_enforcement_helpers.cjs Extend with enforceMax, enforceTitlePrefix, enforceRequiredLabels
actions/setup/js/safe_outputs_handlers.test.cjs Add negative enforcement test cases

References

  • Test results: gh-aw-mcpg#2746
  • Handler registry (constraints source of truth): pkg/workflow/compiler_safe_outputs_config.go L147-595
  • Handler config env var generation: pkg/workflow/compiler_safe_outputs_config.go L863-925
  • Runtime handler execution: actions/setup/js/safe_outputs_handlers.cjs

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions