Skip to content

fix: narrow exception in script permission check from Exception to OSError/PermissionError - #3916

Open
Quratulain-bilal wants to merge 2 commits into
github:mainfrom
Quratulain-bilal:fix/script-chmod-narrow-exception
Open

fix: narrow exception in script permission check from Exception to OSError/PermissionError#3916
Quratulain-bilal wants to merge 2 commits into
github:mainfrom
Quratulain-bilal:fix/script-chmod-narrow-exception

Conversation

@Quratulain-bilal

Copy link
Copy Markdown
Contributor

Problem

Bare except Exception silently swallows all errors during script permission checks.

Fix

Narrow to (OSError, PermissionError) which are the realistic failure modes for file open/read operations.

Testing

  • Verified scripts with shebang get executable permissions
  • Verified scripts without shebang are skipped

…Error/PermissionError

Bare 'except Exception' silently swallows all errors. Narrow to
(OSError, PermissionError) which are the realistic failure modes
for file open/read operations.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Narrows exception handling during script shebang checks.

Changes:

  • Replaces broad Exception handling with filesystem-specific exceptions.
Show a summary per file
File Description
src/specify_cli/__init__.py Narrows script-read exception handling.

Review details

Tip

Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

if f.read(2) != b"#!":
continue
except Exception:
except (OSError, PermissionError):
…Error/PermissionError

Narrow except Exception to (OSError, PermissionError) which are the
realistic failure modes for file open/read operations during shebang
checks.

Add regression tests verifying:
- OSError during open() is skipped (not propagated)
- Unexpected RuntimeError propagates to outer failure handler
- Normal shebang scripts get executable bits (POSIX only)
- Scripts without shebang are skipped (POSIX only)

Co-authored-by: GitHub Copilot (model: mimo-v2.5-free, supervised)
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.

3 participants