Skip to content

fix: accept noDefaultAllow/noDefaultDeny aliases in bashConfig#474

Closed
buger wants to merge 1 commit intomainfrom
fix/bash-config-nodefault-alias
Closed

fix: accept noDefaultAllow/noDefaultDeny aliases in bashConfig#474
buger wants to merge 1 commit intomainfrom
fix/bash-config-nodefault-alias

Conversation

@buger
Copy link
Collaborator

@buger buger commented Mar 4, 2026

Summary

  • Accept visor's noDefaultAllow/noDefaultDeny field names as aliases for probe's disableDefaultAllow/disableDefaultDeny in bashTool()
  • Visor passes bashConfig from workflow YAML directly to probe without field-name mapping, so probe needs to accept both conventions

Test plan

  • Verify bashConfig: { noDefaultAllow: true } disables the default allow list (same as disableDefaultAllow: true)
  • Verify existing disableDefaultAllow/disableDefaultDeny usage continues to work

🤖 Generated with Claude Code

Visor's BashConfig uses noDefaultAllow/noDefaultDeny field names, but
probe only accepted disableDefaultAllow/disableDefaultDeny. Accept both
so workflow YAML using visor's convention works without a separate
mapping step.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@buger buger closed this Mar 4, 2026
@probelabs
Copy link
Contributor

probelabs bot commented Mar 4, 2026

Summary

This PR adds backward compatibility for field name aliases in bashConfig. The bashTool() function now accepts both:

  • Probe convention: disableDefaultAllow / disableDefaultDeny
  • Visor convention: noDefaultAllow / noDefaultDeny

This enables Visor to pass bashConfig from workflow YAML directly to probe without field-name mapping.

Files Changed Analysis

File Status Additions Deletions
npm/src/tools/bash.js Modified 2 2

Pattern: Simple OR fallback pattern (a || b) to accept either field name convention.

Architecture & Impact Assessment

What this PR accomplishes: Enables interoperability between Visor and Probe by accepting both naming conventions for bash permission configuration.

Key technical changes:

  • Lines 48-49 in bash.js: Changed from direct property access to fallback pattern
  • disableDefaultAllow: bashConfig.disableDefaultAllow || bashConfig.noDefaultAllow
  • disableDefaultDeny: bashConfig.disableDefaultDeny || bashConfig.noDefaultDeny

Affected components:

flowchart LR
    A[Visor Workflow YAML] -->|bashConfig| B[bashTool]
    B -->|accepts both| C[BashPermissionChecker]
    C -->|uses| D[Permission Evaluation]
    
    subgraph "Field Name Aliases"
        E[noDefaultAllow]
        F[disableDefaultAllow]
        G[noDefaultDeny]
        H[disableDefaultDeny]
    end
    E -.->|alias| F
    G -.->|alias| H
Loading

Scope Discovery & Context Expansion

Direct impact: npm/src/tools/bash.js - bashTool() function and BashPermissionChecker initialization.

Related files to consider:

  • npm/src/tools/bash.js - Contains BashPermissionChecker class definition (same file)
  • Tests for bash tool permissions (if exist in npm/test-*.js files)
  • Documentation for bashConfig options

Potential follow-up:

  • Consider documenting both field name conventions in API docs
  • Add unit tests for alias acceptance
  • Consider logging a deprecation warning if Visor-style names are used (future enhancement)

Labels

  • tags.review-effort: 1 (trivial - 2 line change, clear intent)
  • tags.label: bug (fixes interoperability issue with Visor integration)
Metadata
  • Review Effort: 1 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-03-04T09:34:11.673Z | Triggered by: pr_opened | Commit: a01bd8d

💡 TIP: You can chat with Visor using /visor ask <your question>

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant