Skip to content

Conversation

@alirezarezvani
Copy link
Owner

Yamllint creates false positives when it sees ** markdown in bash strings inside workflow files, incorrectly interpreting them as YAML syntax.

Since the workflow schemas are still validated by check-jsonschema, this is safe to disable.

@alirezarezvani alirezarezvani force-pushed the fix/yaml-lint-errors branch 2 times, most recently from 7f2eeb5 to 20b89bf Compare November 6, 2025 11:08
@alirezarezvani alirezarezvani merged commit ffbf877 into main Nov 6, 2025
3 of 4 checks passed
@alirezarezvani alirezarezvani deleted the fix/yaml-lint-errors branch November 6, 2025 11:11
@github-actions
Copy link

github-actions bot commented Nov 6, 2025

🔒 Security Audit (Claude)

Severity summary: [Critical: 0, High: 0, Medium: 1, Low: 1]


Findings

  1. [Medium] [.github/workflows/ci-quality-gate.yml:59-66] – Disabled YAML validation removes security controls

    • Risk: Disabling yamllint and check-jsonschema removes automated detection of:
      • YAML syntax errors that could cause workflow failures
      • Schema violations in GitHub Actions workflows
      • Potential injection points in workflow expressions
      • Malformed workflow configurations
    • Recommendation:
      • If false positives are the issue, configure yamllint to ignore specific patterns rather than disabling entirely
      • Example: Use .yamllint config file with custom rules to exclude bash string patterns
      • Keep check-jsonschema enabled as it validates against GitHub's official workflow schema
      • Alternative: Use yamllint --no-warnings to suppress non-critical issues while keeping error detection
  2. [Low] [generated-prompts/*] – Mass file deletion (4,001 lines)

    • Risk: Large deletions can mask malicious changes in PR reviews
    • Recommendation:
      • Verified these are example files being cleaned up (moving to different naming convention based on directory contents)
      • No security impact - examples moved to non-example-N naming pattern
      • Good practice: Consider separate "cleanup" PRs from functional changes for easier review

Notes:

  • Scope: PR diff only (6 files changed)
  • Standards: OWASP Top 10, GitHub Actions security best practices
  • Primary Concern: Removing automated validation increases risk of future workflow vulnerabilities
  • Recommended Action: Reconfigure yamllint rules instead of disabling validation entirely

Justification Review:
The PR description states yamllint creates false positives on ** markdown in bash strings, and that check-jsonschema still validates schemas. However:

  • Both tools were disabled, not just yamllint
  • Check-jsonschema is commented out but PR says it's "still validated" - this appears incorrect
  • Suggest enabling check-jsonschema at minimum, as it catches structural workflow issues

@claude
Copy link

claude bot commented Nov 6, 2025

🔍 Pull Request Review

Summary

This PR disables yamllint and check-jsonschema validation in the CI quality gate. The author's justification is that yamllint creates false positives when encountering ** markdown syntax in bash strings within workflow files.


📊 Review Findings

✅ Positive Aspects

  1. Valid Problem Identification: The issue is real - I confirmed that 10 workflow files contain ** markdown in bash strings (for bold formatting in GitHub comments), which can trigger yamllint warnings.

  2. Clear Documentation: The PR body and inline comments clearly explain the rationale.

  3. Minimal Impact: Only removes 2 validation steps while keeping other checks (Python syntax, safety audit, markdown links).


⚠️ Concerns & Recommendations

🔴 Critical: Loss of Schema Validation

Issue: Disabling check-jsonschema removes GitHub workflow schema validation entirely, not just yamllint rules.

Impact:

  • No validation that workflows conform to GitHub Actions schema
  • Could allow invalid workflow syntax to be merged
  • Breaking changes could reach production undetected

Evidence: The disabled step validates ALL workflow files:

check-jsonschema --builtin-schema github-workflows .github/workflows/*.yml

Recommendation: Keep schema validation enabled. Schema validation doesn't have the same false positive issue as yamllint.


🟡 Medium: Alternative Solutions Not Explored

Issue: Disabling the entire check is the most aggressive solution.

Better Alternatives:

Option 1: Configure yamllint to ignore specific rules

- name: YAML lint (.github/workflows)
  run: |
    yamllint -d '{
      extends: default, 
      rules: {
        line-length: {max: 160},
        key-duplicates: enable,
        quoted-strings: disable,
        truthy: disable
      }
    }' .github/workflows

Option 2: Use inline yamllint disable comments
In problematic workflow files:

# yamllint disable-line rule:whatever-is-triggering
echo "**Bold text**"

Option 3: Separate the two validations

- name: Validate GitHub workflow schemas
  run: |
    check-jsonschema --builtin-schema github-workflows .github/workflows/*.yml

# yamllint disabled due to false positives with markdown in bash
# - name: YAML lint
#   run: yamllint .github/workflows

Recommendation: Implement Option 3 - keep schema validation, only disable yamllint.


🟢 Low: Deleted Files Not Explained

Issue: The PR deletes 5 large documentation files (4,007 lines total):

  • generated-prompts/example-1-basic-code-reviewer.md
  • generated-prompts/example-2-intermediate-data-analyst.md
  • generated-prompts/example-3-advanced-system-architect.md
  • generated-prompts/example-4-expert-research-analyst.md
  • generated-prompts/example-5-master-strategic-consultant.md

Question: Are these deletions intentional? The PR title/description doesn't mention removing documentation.

Recommendation: If intentional, explain why in the PR description. If accidental, revert these deletions.


🎯 Recommended Changes

Must Fix (Blocking)

  1. Re-enable schema validation separately from yamllint
  2. Explain documentation deletions or revert them

Suggested Improvements

  1. Configure yamllint rules instead of full disable (if false positives persist)
  2. Add inline disable comments to specific problematic lines

🔒 Security & Performance

Security: No security concerns - validation tools installation remains
Performance: Minimal improvement (~5-10 seconds faster CI)
⚠️ Risk: Removing schema validation increases risk of invalid workflows


📝 Code Quality

Best Practices Alignment:

  • ❌ Disabling validation contradicts repository's quality standards
  • ❌ CONTRIBUTING.md line 48 mentions "YAML Frontmatter" standards requiring validation
  • ✅ Clear commenting of disabled code

Test Coverage: N/A - workflow configuration change


💡 Alternative Approach (Recommended)

# Keep schema validation - no false positives
- name: Validate GitHub workflow schemas  
  run: |
    check-jsonschema --builtin-schema github-workflows .github/workflows/*.yml

# Only disable yamllint if needed (try configuration first)
- name: YAML lint (.github/workflows)
  run: |
    # Disable specific rules that trigger on markdown
    yamllint -d '{extends: default, rules: {line-length: {max: 160}, quoted-strings: disable}}' .github/workflows

🎬 Verdict

Status: ⚠️ Request Changes

Blockers:

  1. Schema validation should be kept (separate from yamllint)
  2. Documentation deletions need explanation

Effort to Fix: Low (5-10 minutes)


Summary: The core problem (yamllint false positives) is valid, but the solution is too broad. Keep schema validation active and only disable yamllint if configuration tweaks don't resolve the issue. Also clarify the documentation file deletions.

Happy to help iterate on this! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants