Skip to content

t4273: fix routine-helper quality-debt from PR #4251 review#4292

Merged
alex-solovyev merged 1 commit intomainfrom
bugfix/4273-routine-helper-quality-debt
Mar 13, 2026
Merged

t4273: fix routine-helper quality-debt from PR #4251 review#4292
alex-solovyev merged 1 commit intomainfrom
bugfix/4273-routine-helper-quality-debt

Conversation

@alex-solovyev
Copy link
Copy Markdown
Collaborator

Summary

Closes #4273

Addresses all three HIGH-severity findings from PR #4251 review feedback on .agents/scripts/routine-helper.sh.

Two of the three findings were already resolved in commit c2bb03f (merged in PR #4251). This PR adds the remaining gap: the validate_routine_name charset was missing dot (.) support, which the coderabbit suggestion explicitly included (^[A-Za-z0-9._-]+$).

Findings

✅ HIGH (gemini): XML injection — ${command} unescaped in plist heredoc

Resolved in c2bb03f. An xml_escape() helper was added and applied to $command before plist interpolation:

command_escaped=$(xml_escape "$command")
# <string>${command_escaped}</string>

✅ HIGH (coderabbit): ROUTINE_NAME not validated before use in paths/labels

Resolved in c2bb03f + this PR. validate_routine_name() enforces ^[A-Za-z0-9._-]+$ (dot added in this PR to match the suggested regex) and is called in parse_common_args() before any path interpolation.

✅ HIGH (coderabbit): XML escaping needed for all dynamic values in plist heredoc

Resolved in c2bb03f. All five dynamic values are pre-escaped:

label_escaped=$(xml_escape "sh.aidevops.routine-${ROUTINE_NAME}")
command_escaped=$(xml_escape "$command")
home_escaped=$(xml_escape "$HOME")
env_path_escaped=$(xml_escape "/bin:/usr/bin:/usr/local/bin:/opt/homebrew/bin:${PATH}")
log_path_escaped=$(xml_escape "${HOME}/.aidevops/logs/routine-${ROUTINE_NAME}.log")

Verification

shellcheck .agents/scripts/routine-helper.sh
# → zero issues

…stion

The validate_routine_name charset was ^[A-Za-z0-9_-]+$ but the coderabbit
review (issue #4273) suggested ^[A-Za-z0-9._-]+$ to allow names like
'seo.weekly'. Update the regex and error message to match.

All three HIGH findings from PR #4251 review are now fully resolved:
- validate_routine_name enforces safe charset (this commit adds dot support)
- xml_escape() applied to all plist <string> values (c2bb03f)
- ROUTINE_NAME validated before path/label interpolation (c2bb03f)

shellcheck: zero issues
@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions github-actions bot added the bug Auto-created from TODO.md tag label Mar 12, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 12, 2026

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • no-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6edfaab8-66ce-46bf-8072-2230e4efc806

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/4273-routine-helper-quality-debt
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 405 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Thu Mar 12 22:59:27 UTC 2026: Code review monitoring started
Thu Mar 12 22:59:28 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 405

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 405
  • VULNERABILITIES: 0

Generated on: Thu Mar 12 22:59:30 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@sonarqubecloud
Copy link
Copy Markdown

@alex-solovyev alex-solovyev merged commit 7295e2a into main Mar 13, 2026
19 checks passed
@alex-solovyev alex-solovyev deleted the bugfix/4273-routine-helper-quality-debt branch March 13, 2026 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Auto-created from TODO.md tag

Projects

None yet

Development

Successfully merging this pull request may close these issues.

quality-debt: .agents/scripts/routine-helper.sh — PR #4251 review feedback (high)

1 participant