[MohammedAlkindi]

_EXECUTABLE_SUFFIXES (nested_artifacts.py:51) lists .js and .rb but not .mjs, .cjs, .jsx, .tsx, .mts, .cts, .php* or .rake, so is_executable_content returns False for files the scanner's own language maps already treat as script source: _JAVASCRIPT_EXTENSIONS (static_patterns_output_handling.py:76) lists all eight JS/TS forms, and _LANG_BY_EXT (static_patterns_deserialization.py:47) maps .php* and .rake.

That flag gates more than a label. mcp_least_privilege.py:481 returns early as a docs-only skill when nothing is executable, and nested_artifacts.py:984 sets concealed_executable only when it is true.

Three skills with byte-identical script bodies, differing only in extension, scanned with --no-llm. Before:

helper.rb    executable=True    score 19
helper.php   executable=False   score 15
helper.mjs   executable=False   score 15

After, all three are executable at score 19.

Those bodies deliberately carry no shebang. The magic-byte fallback already rescues shebang'd files, and the 0o111 fallback rescues anything carrying the exec bit, so what this changes is suffix-only scripts.

Unit suite 3943 to 3953 passed, the extra ten being the new tests, with an identical 23-failure set either side (pre-existing here: Windows symlink and release-tooling). ruff check and ruff format --check clean.

Added only suffixes an existing analyzer already recognises. .psm1, .pm, .vbs and .wsf are deliberately left out. Found by inspection, not a user report.

[mohgupta-ship-it]

LGTM, waiting on rebase to pass

[mohgupta-ship-it]

Powered by Codex: I checked the suffix-only additions against both SkillSpector's existing analyzer routing and the actual runtime semantics.

Short answer: these are all script-bearing / executable-capable suffixes, but they are not all unconditionally or directly executable merely because of the suffix.

• .cjs and .mjs are the strongest cases: Node treats them as explicit CommonJS and ES-module runtime formats, respectively (Node.js packages documentation).
• .cts and .mts can run directly on sufficiently new Node versions when they use supported erasable TypeScript syntax; otherwise they require a compiler or loader. Node maps them to CommonJS and ESM respectively (Node.js TypeScript documentation).
• .jsx and .tsx are toolchain-dependent source. JSX normally requires transformation, and stock Node explicitly does not support .tsx (TypeScript JSX documentation).
• .php is executable through PHP; .php3, .php4, .php5, and .phtml are legacy/configuration-dependent web-handler signals rather than universally executable extensions. PHP documents that server configuration controls which suffixes are parsed as PHP (PHP manual).
• .rake contains executable Ruby and is loaded by Rake from conventional task directories (Rake documentation).

That means the PR reasonably closes an internal policy mismatch: these suffixes were already routed through SkillSpector's script analyzers, while is_executable_content() failed to mark them as executable-capable. It should still be described as a conservative security classification, not proof that a given file will execute on every host.

One concrete false-positive edge remains: Path(path).suffix classifies types.d.cts and types.d.mts as executable even though those are TypeScript declaration files, not runtime implementations. I recommend explicitly excluding .d.cts and .d.mts and adding regression tests. Apart from that declaration-file case, I did not find an added suffix that is wholly unrelated to executable code.

[mohgupta-ship-it]

One concrete false-positive edge remains: Path(path).suffix classifies types.d.cts and types.d.mts as executable even though those are TypeScript declaration files, not runtime implementations. I recommend explicitly excluding .d.cts and .d.mts and adding regression tests. Apart from that declaration-file case, I did not find an added suffix that is wholly unrelated to executable code.

Please handle this @MohammedAlkindi

[rng1995]

[SkillSpector Review]

Changes requested at head a4bb9fc47f1f5069a576b8cb42c3ce53d934a60f.

• src/skillspector/nested_artifacts.py:60: classification uses Path.suffix, so adding .cts and .mts also marks TypeScript declaration files such as types.d.cts and types.d.mts as executable even though those formats contain declarations and are not runtime implementations. Explicitly exclude the declaration forms before the suffix check and add regression tests for both. This matches the still-unresolved current-head review thread at tests/nodes/test_nested_artifacts.py:836.

Required CI is green, but an active Changes Requested review/unresolved thread, this classification issue, and the BEHIND merge state block merge.

[MohammedAlkindi]

Done in 80c271b. _DECLARATION_SUFFIXES = (".d.cts", ".d.mts") gates the suffix branch only, so
a declaration file carrying a shebang or the executable bit still classifies as executable via
magic bytes or mode.

Regression test is parametrized over both suffixes. Fail-before on the unmodified source:
AssertionError: assert True is False for each; both pass after. The existing plain-suffix
test still asserts True for helper.cts and helper.mts, so nothing widened.

Left .d.ts alone deliberately: .ts predates this PR and neither of you raised it.

[rng1995]

[SkillSpector Review]

Re-reviewed current head 80c271b5105340571550f351c8b433a9390b7c1e. The prior TypeScript declaration-file false positive is resolved: .d.cts and .d.mts are excluded only from suffix-based classification, while shebang and executable-mode detection remain intact, and regressions cover both declarations and the ordinary executable suffixes. I found no remaining required code or test change.

All required checks pass. Do not merge yet: GitHub reports BEHIND, and the addressed declaration-file thread remains unresolved with an active Changes Requested review from mohgupta-ship-it.

[rng1995]

@MohammedAlkindi - Please address and resolve the review comments. Further resolve merge conflict, so that we can merge the PR.

[MohammedAlkindi]

Withdrawing this, on two measurements.

Every suffix here already landed in #548, so the diff against main is empty apart from the exclusion.

And the requested .d.cts/.d.mts exclusion is unsafe. Node runs those files: node evil.d.cts executes, and node t.d.ts executes under type stripping. The .d. infix is a TypeScript convention, not a loader rule. With the exclusion, evil.d.cts holding execSync(...) with no shebang and no exec bit goes True to False. At build_context.py:715 and :1319 the call passes b"" and no mode, so suffix is the only signal there and nothing falls back.

Renaming payload.cts to payload.d.cts would evade the scan. The false positive is real, but this trades it for a false negative.