Skip to content

Conversation

@adityasharad
Copy link
Collaborator

The range A-aa-z was too permissive and includes special characters between Z and a.
Low impact, but fix to address an internally reported code scanning alert.

The range `A-aa-z` was too permissive and
includes special characters between `Z` and `a`.
Low impact, but fix to address an internally
reported code scanning alert.
@adityasharad adityasharad requested review from a team and Copilot June 24, 2025 17:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR tightens regex character class ranges in the QLDoc parsing script to remove unintended special characters and address a security alert.

  • Updated three regex patterns to use [A-Za-z0-9] instead of the overly broad [A-aa-z0-9].
  • No change in behavior beyond narrowing allowed class name characters.

instruction_class_re = re.compile(r'^class (?P<name>[A-aa-z0-9]+)Instruction\s') # Declaration of an `Instruction` class
opcode_base_class_re = re.compile(r'^abstract class (?P<name>[A-aa-z0-9]+)Opcode\s') # Declaration of an `Opcode` base class
opcode_class_re = re.compile(r'^ class (?P<name>[A-aa-z0-9]+)\s') # Declaration of an `Opcode` class
instruction_class_re = re.compile(r'^class (?P<name>[A-Za-z0-9]+)Instruction\s') # Declaration of an `Instruction` class
Copy link

Copilot AI Jun 24, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider extracting the common character class [A-Za-z0-9]+ into a shared constant to reduce duplication and simplify future updates.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

LGTM

@adityasharad adityasharad merged commit 1c567b9 into github:main Jun 24, 2025
6 checks passed
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.

2 participants