Skip to content

Comments

feat(phase2): T007/T008/T067 + pipeline bugfixes — subprocess utils, logger, PowerShell security fix#6

Merged
nsalvacao merged 3 commits intomainfrom
001-cli-plugins-base
Feb 13, 2026
Merged

feat(phase2): T007/T008/T067 + pipeline bugfixes — subprocess utils, logger, PowerShell security fix#6
nsalvacao merged 3 commits intomainfrom
001-cli-plugins-base

Conversation

@nsalvacao
Copy link
Owner

@nsalvacao nsalvacao commented Feb 13, 2026

Summary

  • T007 src/lib/subprocess_utils.py — shared safe subprocess wrapper (list-form only, no shell=True, SAFE_ENV, UTF-8, structured result)
  • T008 src/lib/logger.py — stdlib-only logging setup via CLI_PLUGINS_LOG_LEVEL env var; zero external packages (constitution §Zero Dependencies)
  • T067 🔒 Security fix: executor._build_command PowerShell injection — replaced raw ' '.join(command) with & call operator + individually single-quoted args via _quote_ps_arg() (single quotes escaped as '' per PowerShell convention)
  • Pipeline bugfixes (surfaced by jq E2E test): len(state.errors) on int → direct use; meta string values cast to int for %d logger format

Test plan

  • 24 new unit tests (TDD Red→Green): test_subprocess_utils.py, test_logger.py, test_executor_security.py
  • 127/127 tests pass — zero regressions
  • ruff check clean on all new/modified files
  • E2E validated: cli-crawler jq | generate-plugin → 24 flags extracted correctly, SKILL.md + references/ structure generated
  • .venv-wsl created for WSL environment (AGENTS.md dual-OS strategy)

Phase 2 progress

Task Status
T006 + T044 (schema reconciliation) ✅ prior
T007 subprocess_utils.py ✅ this PR
T008 logger.py ✅ this PR
T067 PowerShell injection fix ✅ this PR
T028/T029/T040/T041/T042/T043/T045/T046 ⏳ next

🤖 Generated with claude-flow

nsalvacao and others added 2 commits February 13, 2026 16:58
@nsalvacao @ruvnet
feat(phase2): implement T007/T008/T067 — subprocess utils, logger, Po…
e48f180 
…werShell fix

T007: src/lib/subprocess_utils.py — safe subprocess wrapper (list-form only,
no shell=True, SAFE_ENV, UTF-8, timeout → structured result)

T008: src/lib/logger.py — stdlib-only logging setup; CLI_PLUGINS_LOG_LEVEL
env var; no external packages (constitution §Zero Dependencies)

T067 (security): fix executor._build_command PowerShell injection vector.
Old: ' '.join(command) passed raw to -Command → args with spaces split.
New: & call operator with individually single-quoted args via _quote_ps_arg().
Single quotes escaped as '' (PowerShell convention).

Tests: 24 new unit tests (TDD Red→Green); 127 total pass; ruff clean.

Co-Authored-By: claude-flow <ruv@ruv.net>
@nsalvacao @ruvnet
fix(pipeline): resolve two runtime bugs found during E2E testing
32df1ef 
- parse_errors: state.errors is int, len() was invalid → use directly
- logger.info %d format: meta values are strings → cast to int()

Both were pre-existing bugs surfaced by the jq E2E test run.
127/127 tests pass; ruff clean.

Co-Authored-By: claude-flow <ruv@ruv.net>
@gemini-code-assist
Copy link

gemini-code-assist bot commented Feb 13, 2026

Summary of Changes

Hello @nsalvacao, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request delivers essential infrastructure improvements by integrating robust subprocess and logging utilities, crucial for the project's stability and adherence to the 'Zero Dependencies' principle. A significant security vulnerability related to PowerShell command injection has been patched, enhancing the application's resilience. Additionally, minor pipeline adjustments ensure correct data handling and logging, contributing to overall system reliability.

Highlights

  • Core Utilities: Introduced src/lib/subprocess_utils.py for safe subprocess execution, enforcing list-form arguments, shell=False, and a SAFE_ENV to prevent injection and ensure deterministic output. Also added src/lib/logger.py for a standard library-only logging setup, configurable via CLI_PLUGINS_LOG_LEVEL.
  • Security Fix (T067): Addressed a PowerShell injection vulnerability in executor._build_command by replacing raw string joining with a secure method using PowerShell's & call operator and individually single-quoted arguments, with proper escaping for embedded single quotes.
  • Pipeline Bugfixes: Resolved issues in src/crawler/pipeline.py where len(state.errors) was incorrectly applied to an integer, and ensured meta string values are correctly cast to integers for logger formatting.
  • Task Tracking: Updated specs/001-cli-plugins-base/tasks.md to mark tasks T006, T044, T007, T067, and T008 as completed.
Changelog
  • specs/001-cli-plugins-base/tasks.md
    • Marked several phase 2 tasks (T006, T044, T007, T067, T008) as completed.
  • src/crawler/executor.py
    • Added a helper function _quote_ps_arg to safely single-quote PowerShell arguments, escaping embedded single quotes.
    • Modified _build_command to use the PowerShell & call operator and _quote_ps_arg for each argument, preventing command injection on Windows.
  • src/crawler/pipeline.py
    • Updated logging statements to explicitly cast meta values (total_commands, total_flags) to integers for correct formatting.
    • Corrected the handling of parse_errors by directly using state.errors instead of len(state.errors).
  • src/lib/logger.py
    • Added a new module logger.py to provide a standard library-only logging setup, configurable via CLI_PLUGINS_LOG_LEVEL.
  • src/lib/subprocess_utils.py
    • Added a new module subprocess_utils.py implementing a safe subprocess wrapper run_safe with enforced rules like list-form commands, shell=False, and a SAFE_ENV.
  • tests/unit/test_executor_security.py
    • Added new unit tests to verify the PowerShell command injection fix in executor._build_command, including tests for argument quoting and escaping.
  • tests/unit/test_logger.py
    • Added new unit tests for the lib/logger.py module, covering logger creation, default levels, environment variable configuration, and singleton behavior.
  • tests/unit/test_subprocess_utils.py
    • Added new unit tests for the lib/subprocess_utils.py module, covering basic command execution, timeout handling, command not found scenarios, and shell injection prevention.
Activity
  • 24 new unit tests were added across test_subprocess_utils.py, test_logger.py, and test_executor_security.py.
  • All 127 existing tests passed, indicating zero regressions.
  • ruff check was run and passed cleanly on all new and modified files.
  • End-to-end validation was performed using cli-crawler jq | generate-plugin, successfully extracting 24 flags and generating SKILL.md and references/ structure.
  • A .venv-wsl environment was created to support WSL development, aligning with the AGENTS.md dual-OS strategy.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@nsalvacao nsalvacao requested a review from Copilot February 13, 2026 17:04
Copilot AI reviewed Feb 13, 2026
View reviewed changes
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 implements three Phase 2 tasks (T007, T008, T067) and includes pipeline bugfixes. It introduces safe subprocess utilities, stdlib-only logging infrastructure, and a critical security fix for PowerShell command injection. However, the PR accidentally introduces a critical bug in the ANSI escape sequence regex.

Changes:

  • Added subprocess_utils.py with safe subprocess wrapper enforcing list-form commands, no shell=True, and SAFE_ENV
  • Added logger.py with stdlib-only logging configured via CLI_PLUGINS_LOG_LEVEL environment variable
  • Fixed PowerShell injection vulnerability in executor._build_command using single-quoted arguments and the & call operator
  • Fixed pipeline.py bugs: corrected state.errors usage (int, not list) and meta dict access for logger formatting

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/lib/subprocess_utils.py New safe subprocess wrapper with SAFE_ENV, timeout handling, and UTF-8 encoding
src/lib/logger.py New stdlib-only logging module with environment-based configuration
src/crawler/executor.py PowerShell security fix with _quote_ps_arg helper; critical bug introduced in ANSI_RE regex
src/crawler/pipeline.py Bugfixes for meta dict access and state.errors int usage
tests/unit/test_subprocess_utils.py 9 unit tests for subprocess utilities (some Unix-specific)
tests/unit/test_logger.py 5 unit tests for logging configuration
tests/unit/test_executor_security.py 10 security tests for PowerShell command injection prevention
specs/001-cli-plugins-base/tasks.md Marked T006, T007, T008, T044, T067 as complete

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nsalvacao @ruvnet
fix(pr6): address Copilot review + fix CI lint failure
497b162 
- ruff format test_executor_security.py (CI lint was failing on this)
- test_subprocess_utils: replace ["sleep","10"] and ["env"] with
  cross-platform python3 equivalents (Copilot comments 3 & 4)
- test_logger: consolidate dual import into single `import lib.logger
  as logger_module` (Copilot comment 5)
- subprocess_utils: add cross-reference comment for SAFE_ENV duplication
  with executor.py (Copilot comment 2)

All 127 tests pass. ruff check + ruff format --check clean.

Co-Authored-By: claude-flow <ruv@ruv.net>
@nsalvacao nsalvacao merged commit 3feb550 into main Feb 13, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@nsalvacao