Skip to content

fix(reformat): reformat overwrites content#4672

Merged
scudette merged 2 commits intoVelocidex:masterfrom
nullifysecurity:fix/reformat-overwrite
Feb 21, 2026
Merged

fix(reformat): reformat overwrites content#4672
scudette merged 2 commits intoVelocidex:masterfrom
nullifysecurity:fix/reformat-overwrite

Conversation

@nullifysecurity
Copy link
Contributor

This pull request fixes an issue where the current reformat logic could unintentionally overwrite content following the query field in an artifact. This most commonly affected artifacts that define additional fields such as notebook or column_types.

For example, given the following artifact with a notebook field:

name: Notebook
sources:
- query: |
    SELECT A,B,C
    FROM scope()
  notebook:
    - name: Test
      type: vql_suggestion
      template: |
        SELECT * FROM scope()

Running the reformat command (either via CLI or the GUI) would remove the notebook: line. Subsequent reformats would continue stripping content, producing an invalid artifact such as:

name: Notebook
sources:
- query: |
    SELECT A,
           B,
           C
    FROM scope()
    - name: Test
      type: vql_suggestion
      template: |
        SELECT * FROM scope()

Error: While parsing VQL at line 8: unexpected token "-"

The root cause was an off-by-one error in the mutation logic applied to the query block, which caused the reformat operation to extend beyond the intended range. While addressing this, I also identified an issue with how original_end_line was calculated for chomping indicator nodes (|-). Because these nodes omit the trailing newline, splitting on \n resulted in an incorrect end line calculation.

Additionally, the current_mu.err value was never checked when applying mutations that only had a length of one. This has also been resolved by adding an additional check at the beginning of the line iteration.

This change corrects these issues and adds regression tests to verify that:

  • Content following a query block is preserved.
  • Running reformat multiple times produces stable, idempotent output.

Please let me know if you'd like further clarification, or if there are any changes you'd like made.

I am also considering a refactor of the artifacts reformat to call a reformat VQL function which wraps this functionality, following the pattern that CLI commands are just wrappers around VQL. Let me know if this is also something you'd be interested in me contributing. 😃

@scudette scudette merged commit 9ddb189 into Velocidex:master Feb 21, 2026
5 checks passed
@scudette
Copy link
Contributor

Thanks for this! +1 on the reformat() VQL function idea - it would be very helpful.

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