From 436707f8ff0d56bd817f43c1b66f0f6c36cb616e Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 14 Aug 2026 18:27:06 +0200 Subject: [PATCH] fix(yaml): treat interior `*` and `&` as text, not as alias/anchor indicators yaml_range_has_unsupported rejected `&` and `*` anywhere in a plain scalar, with no positional test at all. So a value containing prose asterisks - `use *emphasis* here`, a kaomoji, a glob inside a description - was refused as if it were an alias. That is one of four constructs that made a real 16 KB hand-maintained Hermes config permanently un-editable by us (#1631). It is not YAML we cannot model; it is YAML we declined to read. An indicator only counts where a NODE begins. The test is now positional and judged by what PRECEDES the character, because the scanned range covers a whole line including its key: in `command: &shared` the `&` is an anchor even though `command:` came first. A node begins at the range start, after a mapping colon, or after a block-sequence dash. Everything else is text. `{` and `}` keep their existing treatment. Empty flow mappings (`key: {}`) are the next item in #1631 and carry their own semantics; bundling them here would mix a positional correction with a value-semantics change. Root-caused by @rg6304, who reproduced it in isolation, read this source, and corrected the maintainer's hypothesis - the constructs I had guessed (inline comments, anchors, `---` separators) appear nowhere in the failing file. TESTING NOTE, recorded because it nearly went wrong: the first version of the regression test put the asterisk in an untouched foreign section and passed with the fix REVERTED - the editor only validates the range it writes, so the test proved nothing. Rewritten onto the entry block, it now fails without the fix (`== -1, expected 0`) and passes with it. The companion test pins the other direction: a LEADING `*` is still an alias and still refused, byte-identically. config_yaml_edit + yaml suites: 117 passed, 0 failed. Signed-off-by: Martin Vogel --- src/cli/config_yaml_edit.c | 33 ++++++++++++++++++++++++++++- tests/test_config_yaml_edit.c | 40 +++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/cli/config_yaml_edit.c b/src/cli/config_yaml_edit.c index 7a05d1ee8..48e2039f4 100644 --- a/src/cli/config_yaml_edit.c +++ b/src/cli/config_yaml_edit.c @@ -1254,23 +1254,54 @@ static int yaml_find_comment(const char *data, size_t start, size_t end, size_t return 0; } +/* `&` and `*` are anchor/alias indicators only where a NODE begins. Once a + * plain scalar has started, they are ordinary characters. + * + * This used to reject them anywhere in the range, so a value containing prose + * asterisks — `use *emphasis* here`, a kaomoji, a glob in a description — was + * refused as if it were an alias. Reported against a real 16 KB Hermes config + * where this was one of four constructs that made the file permanently + * un-editable by us (#1631, root-caused by @rg6304 with an isolated repro). + * + * The test is positional and deliberately conservative: an indicator is only + * honoured as one when nothing has appeared before it in the value. `key: *a` + * is still an alias and still refused; `key: 2 * 3` and `key: a*b` are text. + * `{`/`}` keep their existing treatment — empty flow mappings are a separate + * change with its own semantics. */ static int yaml_range_has_unsupported(const char *data, size_t start, size_t end) { char quote = '\0'; + /* Last significant character seen, so an indicator can be judged by what + * PRECEDES it. The scanned range covers a whole line including its key, so + * "first character in the range" is not the same as "start of a node" — + * in `command: &shared` the `&` is an anchor even though `command:` came + * first. A node begins at the range start, after a mapping colon, or after + * a block-sequence dash. */ + char previous = '\0'; for (size_t i = start; i < end; i++) { bool is_plain = false; if (yaml_scan_quote(data, end, &i, "e, &is_plain) != 0) { return YAML_MATCH; } if (!is_plain) { + previous = 'q'; continue; } char c = data[i]; if (c == '#' && (i == start || data[i - YAML_UNIT] == ' ')) { break; } - if (c == '{' || c == '}' || c == '&' || c == '*') { + if (c == '{' || c == '}') { return YAML_MATCH; } + if (c == '&' || c == '*') { + bool begins_node = previous == '\0' || previous == ':' || previous == '-'; + if (begins_node) { + return YAML_MATCH; + } + } + if (c != ' ' && c != '\t') { + previous = c; + } if (c == '<' && i + YAML_ENTRY_INDENT < end && data[i + YAML_UNIT] == '<' && data[i + YAML_ENTRY_INDENT] == ':') { return YAML_MATCH; diff --git a/tests/test_config_yaml_edit.c b/tests/test_config_yaml_edit.c index 88b15cf5c..3c963747d 100644 --- a/tests/test_config_yaml_edit.c +++ b/tests/test_config_yaml_edit.c @@ -1501,6 +1501,44 @@ TEST(config_yaml_edit_nested_sequence_ambiguity_fails_byte_identically) { } #ifndef _WIN32 +/* #1631: an interior `*` in a written value is ordinary text, not an alias. + * A real 16 KB Hermes config was permanently un-editable because prose + * asterisks in a personality string were read as aliases (root-caused by + * @rg6304 with an isolated repro). + * + * The asterisk is in the ENTRY BLOCK deliberately: that is the range the + * editor validates. An earlier version of this test put it in an untouched + * foreign section, which is never scanned - so it passed with the fix + * reverted and proved nothing. */ +TEST(config_yaml_edit_accepts_interior_asterisk_in_plain_scalar_issue1631) { + const char *original = "existing:\n keep: 1\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, original), 0); + ASSERT_EQ(cbm_yaml_upsert_mapping_entry(fixture.path, "hooks", "cbm", + " value: use *emphasis* and 2 * 3\n"), + 0); + char *after = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(after); + ASSERT_NOT_NULL(strstr(after, "value: use *emphasis* and 2 * 3")); + free(after); + ASSERT_EQ(cbm_unlink(fixture.path), 0); + th_cleanup(fixture.dir); + PASS(); +} +TEST(config_yaml_edit_still_refuses_leading_alias_issue1631) { + const char *original = "existing:\n keep: 1\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, original), 0); + ASSERT(cbm_yaml_upsert_mapping_entry(fixture.path, "hooks", "cbm", " value: *alias\n") != 0); + char *after = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(after); + ASSERT_STR_EQ(after, original); + free(after); + ASSERT_EQ(cbm_unlink(fixture.path), 0); + th_cleanup(fixture.dir); + PASS(); +} + TEST(config_yaml_edit_nested_sequence_rejects_symlink_byte_identically) { const char *original = "hooks:\n pre_llm_call:\n - id: \"other\"\n"; yaml_fixture_t fixture; @@ -1573,6 +1611,8 @@ SUITE(config_yaml_edit) { RUN_TEST(config_yaml_edit_nested_sequence_removes_only_exact_canonical_item); RUN_TEST(config_yaml_edit_nested_sequence_ambiguity_fails_byte_identically); #ifndef _WIN32 + RUN_TEST(config_yaml_edit_accepts_interior_asterisk_in_plain_scalar_issue1631); + RUN_TEST(config_yaml_edit_still_refuses_leading_alias_issue1631); RUN_TEST(config_yaml_edit_nested_sequence_rejects_symlink_byte_identically); #endif }