Skip to content

rules: anchor features offset to section header in rule formatter - #3143

Open
SkxOverKill wants to merge 2 commits into
mandiant:masterfrom
SkxOverKill:fix/capafmt-namespace-features-offset
Open

rules: anchor features offset to section header in rule formatter#3143
SkxOverKill wants to merge 2 commits into
mandiant:masterfrom
SkxOverKill:fix/capafmt-namespace-features-offset

Conversation

@SkxOverKill

@SkxOverKill SkxOverKill commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #3134 by preventing Rule.to_yaml() / capafmt from corrupting rules whose namespace contains the substring features, such as impact/features/persistence.

Relationship to #3135

This PR is functionally a duplicate of the earlier #3135: both address #3134 by anchoring the formatter's search to the actual top-level features: section and both add regression coverage for a metadata description combined with a features namespace.

#3135 predates this PR and has already received maintainer approval. This PR is retained only as a CLA-compliant fallback because #3135 remains blocked on its contributor's CLA. If #3135 becomes mergeable, this PR should be closed in its favor. Credit to @Makeph for the earlier contribution and analysis in #3135.

Root Cause

The formatter located the start of the features block with:

features_offset = doc.find("features")

Because str.find() returns the first matching substring, a namespace containing features caused the offset to point into rule metadata. The subsequent feature-description indentation adjustment then also modified the metadata description, producing invalid YAML.

Implementation

  • Anchor the offset to the top-level YAML section header: \n features:.
  • Add a round-trip regression test containing both namespace: impact/features/persistence and metadata/feature descriptions.
  • Add the required entry under master (unreleased) in CHANGELOG.md.

Verification

python -m pytest tests/test_fmt.py tests/test_rules.py tests/test_rule_cache.py -q
45 passed in 0.35s

python -m ruff check --config .github/ruff.toml capa/rules/__init__.py tests/test_fmt.py
All checks passed!

python -m ruff format --check --config .github/ruff.toml capa/rules/__init__.py tests/test_fmt.py
2 files already formatted

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed

@williballenthin

Copy link
Copy Markdown
Collaborator

@SkxOverKill what is the relation between this PR and #3135?

@github-actions
github-actions Bot dismissed their stale review August 12, 2026 13:59

CHANGELOG updated or no update needed, thanks! 😄

@SkxOverKill

Copy link
Copy Markdown
Contributor Author

Thanks for asking. #3143 is functionally a duplicate of the earlier #3135: both fix #3134 by anchoring the formatter offset to the actual top-level eatures: section, and both add regression coverage for a metadata description in a eatures namespace. #3135 predates this PR and is already approved, but it remains blocked on its contributor's CLA. I have updated this PR to acknowledge that relationship and @Makeph's earlier contribution explicitly. I also added the missing changelog entry and revalidated the formatter/rules/cache tests (45 passed), so #3143 is available strictly as a CLA-compliant fallback. If #3135 becomes mergeable, I am happy for this PR to be closed in its favor.

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.

capafmt breaks description field if namespace contains features

2 participants