fix: add heading hierarchy check and fix set -e crash#22
Conversation
…s.sh
Two issues fixed:
1. **Bug fix**: `((WARNINGS++))` and `((ERRORS++))` with `set -e` causes
the script to exit prematurely when the counter is 0, because bash
treats `((0))` as falsy (exit code 1). Replaced with
`WARNINGS=$((WARNINGS + 1))` which always succeeds.
2. **New check**: Added TYPO3 heading hierarchy validation that detects:
- First section heading not using `=` (h2) — catches the common issue
where sections extracted from a parent page retain their old heading
levels (e.g. `-` for h3 instead of `=` for h2)
- Non-standard underline characters (e.g. `^` instead of `=/-/~`)
- Skipped heading levels (e.g. h2 directly to h4 without h3)
The check follows TYPO3 heading convention:
- h1: `=` above and below (page title)
- h2: `=` below only (sections)
- h3: `-` below only (subsections)
- h4: `~` below only (sub-subsections)
This would have caught the heading issues reported in
TYPO3-Documentation/TYPO3CMS-Guide-HowToDocument#510.
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Summary of ChangesHello, 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 addresses a critical bug in the Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
There was a problem hiding this comment.
Code Review
This pull request fixes a crash caused by set -e and ((VAR++)) arithmetic in bash, and introduces a new feature for validating heading hierarchy in RST files. However, the implementation of this new feature introduces a critical Command Injection and Python Code Injection vulnerability. This is due to the direct interpolation of a shell variable ($file) into a Python script executed via python3 -c, which could allow an attacker to execute arbitrary code. It is recommended to refactor the embedded Python code into a separate file to resolve this security risk and improve maintainability.
Move inline Python code from validate_docs.sh to validate_headings.py to fix command injection vulnerability (shell variable $file was interpolated directly into python3 -c code) and improve maintainability. The filename is now passed securely as sys.argv[1]. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Summary
((WARNINGS++))withset -ecrashes the script when counter is 0 (bash treats((0))as falsy exit code 1). All((VAR++))replaced with safeVAR=$((VAR + 1))arithmetic.=(h2) — the most common mistake when extracting sections into standalone pages^)Motivation
This was discovered during TYPO3-Documentation/TYPO3CMS-Guide-HowToDocument#510 where extracted pages retained their old heading levels. The
validate_docs.shscript ran but never reached the heading check due to theset -e+((0++))crash, and even if it had, no heading hierarchy check existed.Test plan
validate_docs.shagainst a TYPO3 extension with correct headings — should pass-instead of=— should flag itset -ecrash no longer occurs when first warning is encountered