Skip to content

__is_block__, __comments__ and the with_meta keys silently displace attributes of the same name #331

Description

@livingstaccato

Summary

The serializer's metadata is carried in-band: __is_block__, __comments__ and
__inline_comments__ (and __start_line__ / __end_line__ where with_meta
is honoured) go into the same dict as the block's attributes. HCL puts no such
names out of reach — __is_block__ is a valid identifier — so a document that
declares one gets no error and no warning, just a different value or none at
all.

Both directions lose:

import hcl2

hcl2.loads('block "a" {\n  __is_block__ = 99\n  keep = 1\n}\n')
# {'block': [{'"a"': {'__is_block__': True, 'keep': 1}}]}      99 is gone

hcl2.dumps(hcl2.loads('block "a" {\n  __comments__ = 99\n  keep = 1\n}\n'))
# 'block "a" {\n  keep = 1\n}\n'                                the attribute is gone

On read, the marker overwrites the attribute. On write, _is_reserved_key
drops the key because it cannot tell a marker it wrote from an attribute the
document declared. Neither is recoverable by the caller: by the time the dict
exists the distinction is gone.

Reproduction

Released 8.1.3, clean virtualenv:

__is_block__           -> 'block "a" {\n  keep = 1\n}\n'
__comments__           -> 'block "a" {\n  keep = 1\n}\n'
__inline_comments__    -> 'block "a" {\n  keep = 1\n}\n'

(hcl2.dumps(hcl2.loads(...)) for each, with a second attribute keep = 1 to
show the rest of the block survives.)

Why it is worth filing rather than documenting

The keys are a v7 interface that v8 kept, so the shape is hard to change now,
and for resource/variable/module documents the collision will never
happen. But this library is not Terraform-only: it parses any HCL2, and a
generated or machine-written document can carry any identifier. The failure mode
is the bad one — silent, in both directions, with the corrupted dict being what
the caller then acts on.

Suggested shape of the fix

Anything that makes the metadata unambiguous has to take it out of band: a
sidecar mapping, a wrapper object, or a SerializationOptions field naming a
prefix the caller guarantees is unused. All of those change the serialized shape
and belong in a major release, which is why this is filed as a decision to make
rather than a patch to apply.

Two smaller steps are available now and are compatible:

  • On write, raise instead of dropping when a body carries a reserved key whose
    value is not the marker the serializer would have produced. Loud beats silent,
    and a document that genuinely declares __is_block__ = 99 is already outside
    what the current shape can represent.
  • Document the five names as reserved, which they effectively are.

Related


This issue, and the investigation behind it, were produced by an AI assistant (Claude) working on behalf of the author. Please review with that provenance in mind.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions