Skip to content

Expose disallowed_tools input so callers can actually revoke tools from the action baseline #146

Closed

Description

@twistedmelonman

Non-Blocking Review Concern: Expose disallowed_tools input so callers can actually revoke tools from the action baseline

Source: PR author (self-documented limitation in claude-assistant.yml header and README)
Location: .github/workflows/claude-assistant.yml
PR: #145 — feat(claude-assistant): add allowed_tools and model inputs (#145)
Date: 2026-08-17

What was flagged

The PR documents that allowed_tools unions with claude-code-action's tag-mode baseline and therefore can only grant tools, never revoke them. Genuine revocation requires --disallowed-tools, which this workflow does not expose. Both the workflow header and README explicitly say to open an issue if that capability is needed. Worth tracking so a repo that wants a real restriction has a supported path rather than assuming the allowlist removed something.

Context

This issue was automatically created from a non-blocking concern identified
during pre-merge review of PR #145. It was safe to merge but worth tracking.


Created by lib-review-issues.sh

Activity

  1. twistedmelonman commented on Aug 18, 2026

    @twistedmelonman
    MemberAuthor

    Resolved from source — disallowed_tools does subtract

    The precedence documented as UNVERIFIED is now verified, by reading claude-code-action at the exact SHA this workflow pins (9d7150bc, v1.0.193) rather than by a live run. Same method that settled allowed_tools.

    The trace

    1. Tag mode emits an allowlist only — never a denylist.
    src/modes/tag/index.ts:186:

    claudeArgs += ` --permission-mode acceptEdits --allowedTools "${tagModeTools.join(",")}"`;

    Then :189-190 appends the caller's args after it. Grepping the action for disallowedTools: src/modes/tag/index.ts → 0, src/modes/agent/index.ts → 0. Only base-action/src/run-claude.ts mentions it, as a type field.

    This is the decisive fact. There is no competing denylist. Our --disallowed-tools is the only one in play, so there is no precedence contest for it to lose — which was the specific worry.

    2. Our flag survives parsing intact.
    base-action/src/parse-sdk-options.ts:245-266 handles disallowedTools with the same accumulate-and-union logic as allowedTools, reading both --disallowedTools and --disallowed-tools spellings:

    const mergedDisallowedTools = [
      ...new Set([...extraArgsDisallowedTools, ...directDisallowedTools]),
    ];

    3. It reaches the SDK as a first-class option.
    parse-sdk-options.ts:321-322:

    disallowedTools:
      mergedDisallowedTools.length > 0 ? mergedDisallowedTools : undefined,

    Not left in extraArgs — explicitly deleted from there (:265-266) and set on the SDK options object.

    Why the allowed_tools failure mode doesn't recur

    allowed_tools surprised us because tag mode emits its own --allowedTools and the parser unions rather than replaces, so a caller allowlist could only ever ADD. The denylist has no such counterpart: nothing upstream emits one to union with. The union at :262 is between our own extraArgs and direct-input spellings, not against an action-supplied baseline.

    What this does and does not establish

    Established from source: the value is parsed, merged, and handed to the SDK as disallowedTools for the pinned version.

    NOT established here: the SDK's internal deny-beats-allow rule for a tool named in both lists. That is Claude Code CLI behavior, outside this action. Given tag mode emits no denylist, the only way to hit that case is a caller listing the same tool in both of its own inputs — a self-contradictory config, not the revocation path this input exists for.

    Recommendation

    Downgrade the docs from "unproven, confirm with a live run" to verified-against-v1.0.193, matching how allowed_tools is described, and note the version the trace was performed against. This was the last item gating a v3 repoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    tech-debtTechnical debt to address

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions