Skip to content

YAML store converts inline comments to preceding-line comments on encrypt/edit roundtrip #2130

Closed
knowald/sops
#1

Description

@knowald

Hi! I ran into this while setting up unencrypted_comment_regex for a project.

When using unencrypted_comment_regex (or encrypted_comment_regex) with YAML files, inline trailing comments are converted to preceding-line comments after the first encrypt. The feature works correctly - values are encrypted/unencrypted as expected - but the comment position changes permanently.

Input:

DATABASE_HOST: postgres # unencrypted
DATABASE_PORT: "5432" # unencrypted
DATABASE_PASSWORD: secret

After sops encrypt --in-place and sops decrypt:

# unencrypted
DATABASE_HOST: postgres
# unencrypted
DATABASE_PORT: "5432"
DATABASE_PASSWORD: secret

Subsequent sops edit cycles keep the preceding-line format. The docs say "a trailing comment on the same line" is supported, so users expect inline comments to be preserved.

Possible root cause:

Looking at the source, I believe the issue is in stores/yaml/store.go. During parsing (appendYamlNodeToTreeBranch, line 122), value.LineComment is converted to a sops.Comment tree item appended before the TreeItem. On write-back (appendTreeBranch, line 244), accumulated comments are placed as HeadComment on the next key node via addCommentsHead. There doesn't appear to be a path to restore them as LineComment on the original value node.

Expected behavior:

Inline comments should be preserved as inline comments through encrypt/decrypt cycles.

SOPS version: 3.12.2

Thanks for maintaining SOPS!

Activity

  1. knowald commented on Mar 31, 2026

    @knowald
    ContributorAuthor

    If my understanding of the root cause is correct and preserving inline comments as LineComment on the original value node would be an acceptable approach, I'd be happy to open an MR with a proposed fix. Let me know if that would be welcome or if you'd prefer a different direction.

  2. felixfontein commented on Mar 31, 2026

    @felixfontein
    Contributor

    When using unencrypted_comment_regex (or encrypted_comment_regex) with YAML files, inline trailing comments are converted to preceding-line comments after the first encrypt. The feature works correctly - values are encrypted/unencrypted as expected - but the comment position changes permanently.

    This is unrelated to unencrypted_comment_regex or encrypted_comment_regex, but to how SOPS processes comments in general (as you noticed when looking into the YAML store's code).

    This is basically how SOPS has been working for a long time, and changing this requires a good design of how this should work first.

  3. knowald commented on Mar 31, 2026

    @knowald
    ContributorAuthor

    Now that you say it - yeah, you're right. I've run into the comment position shift before with regular YAML comments and somehow completely forgot about it while being focused on the encrypt/unencrypt behavior.

    Would you be open to discussing a fix for the general YAML comment roundtrip issue, or should I take this as "better left untouched" and close the issue?

  4. felixfontein commented on Mar 31, 2026

    @felixfontein
    Contributor

    I'm fine with discussing it. You just have to note that perfect roundtrip preserving won't be possible, since not even the underlying YAML library supports it, and even if it would, I don't think we would adjust the internal format to be too YAML-centric (it also has to work for other formats, i.e. JSON, INI, DotEnv, and in the future TOML and maybe also XML). Generally SOPS' aim isn't to preserve formatting, but to preserve structure. Comments are one big exception; for YAML, comments basically don't count as part of the structure, which is also why most YAML libraries struggle with preserving comments when round-tripping.

  5. knowald commented on Mar 31, 2026

    @knowald
    ContributorAuthor

    Looks like the YAML store already reads HeadComment, LineComment, and FootComment separately during parsing - the go.yaml.in/yaml/v3 library provides all three on yaml.Node. FootComment roundtrips correctly atm because its position in the internal tree preserves the distinction.

    LineComment doesn't as it's placed in the same position as HeadComment, so the two become indistinguishable and everything gets written back as HeadComment.

    I think the minimal fix would be adding a single Inline bool to the Comment struct so the distinction survives the conversion? The YAML store would set it when the source is a LineComment, and restore it during write-back instead of collapsing it into HeadComment. This follows the same pattern as the existing addCommentsHead/addCommentsFoot - just the missing third piece i think.

    Defaulting to false would mean no changes, and only code that sets or reads the flag would be the YAML store's parser and writer.

    Would that kind of scope be acceptable? Happy to put together a PR if so.

    Edit: TOML has the same inline vs preceding-line distinction, so a future store could use the same field.

    Edit2: Another thought. Making Comment an interface with format-specific implementations would be cleaner long-term, but that would be massive - walkBranch, the cipher layer and every store would need changes. As for the Inline flag itself, looking at the current INI store, it could be used if someone adds inline comment support there at some point. DotEnv and JSON are unaffected. XML comments are a different beast though - they're standalone nodes in the DOM tree with no attachment to adjacent elements, so they'd break the current model entirely. That would need the interface rework, but i think that's a separate effort.

  6. added a commit that references this issue on Apr 1, 2026
    4661ac0
  7. knowald commented on Apr 1, 2026

    @knowald
    ContributorAuthor

    I've put together a draft PR as an example of the idea.
    #2131

    It adds the Inline bool, sets it in the YAML store's parser and restores it during write-back via a new helper alongside the existing addCommentsHead/addCommentsFoot helpers. No changes outside the YAML store except the struct field.

    Would love to hear if I'm reading the comment handling right and if this direction works for the project.

  8. felixfontein commented on Apr 7, 2026

    @felixfontein
    Contributor

    I think the minimal fix would be adding a single Inline bool to the Comment struct so the distinction survives the conversion? The YAML store would set it when the source is a LineComment, and restore it during write-back instead of collapsing it into HeadComment.

    After looking at the implementation for some time now, I think this should be fine.

    Edit2: Another thought. Making Comment an interface with format-specific implementations would be cleaner long-term, but that would be massive - walkBranch, the cipher layer and every store would need changes. As for the Inline flag itself, looking at the current INI store, it could be used if someone adds inline comment support there at some point. DotEnv and JSON are unaffected. XML comments are a different beast though - they're standalone nodes in the DOM tree with no attachment to adjacent elements, so they'd break the current model entirely. That would need the interface rework, but i think that's a separate effort.

    I don't think this would be a good idea. SOPS can also use different formats for input than for output, and having to be able to convert between different kind of comments there would make this way too complex. (Also I don't think it would work well with potential future "format plugins".)

  9. added a commit that references this issue on Apr 7, 2026
    42f75be
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions