Repository navigation
Conversation
Signed-off-by: Milad Heydari <[email protected]>
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Reviewed current head d43c946ace50e9e22e1e78ee2bbada002e892a05.
Changes requested: src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py:732 treats a shell command word composed only of a plain parameter as non-dynamic. A plain parameter can still synthesize an executable command name: for example, CMD=printf; $($CMD echo) hello makes the inner expansion produce echo, which the outer substitution executes. In the dangerous analogue, this change causes _is_printf_substitution to stop recognizing the dynamic inner command and silently reports neither a finding nor a parse limit. Preserve fail-closed/dynamic handling when a parameter-only command word has arguments in an executable substitution; narrow the exemption to cases proven non-executable, and add a regression for command-name synthesis.
Hosted required checks pass, but the PR currently has merge conflicts. It is not merge-ready.
|
@iammilaad - Please resolve review comments and merge conflicts to merge the PR. |
|
[SkillSpector Review] Hi @iammilaad, thank you for your contribution and for reporting the false parse-limit error on bare shell variables such as The issue this PR targets, #464, was closed as completed by #507 ("fix: avoid false parser limits and reference accounting failures", merged 2026-09-09), and this branch now conflicts with If we've missed something — for example, a case that still reproduces on current |
Fixes #464