fix(okf): a mapping construct no longer degrades into a string, and there were two routes
T2 gives the mapping class no expressible form by design. Two routes escaped that by parsing "successfully" into the wrong TYPE instead of raising: sources:\n - uri: https://e.com/a -> the string 'uri: https://e.com/a' attester: resource: attesters/x.py -> the string 'resource: attesters/x.py' Only the first was documented (LIMITATIONS.md:43). The inline second colon was found by measurement while closing it -- a real YAML parser refuses that line outright, ours accepted it. Shipping the list half alone would have left a LIMITATIONS rewrite that overclaims. Same consequence either way: a pointer parked in a degraded mapping rides through in a key the `resource` allowlist never inspects, and mode-b import_bundle wrote the merged concept verbatim (WARN). Both now FAIL_SECURE at T2, before the allowlist is reached. The boundary is where YAML puts it, ground-truthed against PyYAML 6.0.3 rather than reasoned: ": " and a trailing ":" are exactly the two shapes where a plain scalar becomes a mapping. A colon carrying neither a space nor a line end opens no mapping -- domain:security and https://e.com:8443/a still parse -- and a quoted scalar is still a scalar. Over-blocking a conformant bundle is itself a failure mode, so the seven admitted shapes get rows of their own. Iron Law: the four rejected rows and both import_bundle rows were written first and seen red (7 failures, each DID NOT RAISE) before okf.py was touched. Suite 792 -> 802. LIMITATIONS stays at 35: the bullet is reworded, not retired -- the restricted grammar is still a limitation, the silent misparse is no longer part of it.
This commit is contained in:
parent
6cd4694613
commit
da30211bc7
3 changed files with 122 additions and 39 deletions
|
|
@ -40,26 +40,29 @@ items; this is the full list, each with the mechanism.
|
|||
(an injection in a directory listing is caught) rather than path-rejecting the
|
||||
conformant bundle. A front-end materialising individual uploads keeps the opposite
|
||||
rule (`allow_reserved=False`): a reserved basename is a listing-shadow and refused.
|
||||
- **OKF frontmatter is a restricted grammar, and a one-key block-sequence item is
|
||||
silently misparsed.** Gate T2 accepts a line-oriented subset deliberately — full
|
||||
YAML is a larger parse-attack surface than a write-time gate needs. Nested mappings
|
||||
and flow collections (`[a, b]`, `{k: v}`) are *rejected outright*, which fails
|
||||
secure. **All three routes to a mapping fail, each on a different rule** — flow
|
||||
(`{k: v}`) on the disallowed value-start indicator, block (`k:\n sub: v`) on the
|
||||
nested-mapping check, and dotted keys (`k.sub: v`) on the key pattern — so the
|
||||
mapping *class* has no expressible form, rather than one form being preferable to
|
||||
another. What survives is scalars and flat lists of strings. The defect is between
|
||||
those two outcomes: a block sequence whose items carry
|
||||
exactly **one** key parses "successfully" into the wrong type —
|
||||
`sources:\n - uri: https://e.com/a` yields the **string** `'uri: https://e.com/a'`,
|
||||
not a mapping, while the same list with two keys per item hard-rejects. A pointer
|
||||
can therefore ride through in a key the `resource` allowlist never inspects
|
||||
(`attester:\n - resource: attesters/sql_equality.py` → WARN), whereas a top-level
|
||||
`resource:` with a relative path correctly fails secure. The shape is not conformant
|
||||
OKF, so a well-formed bundle will not produce it; a malformed or hostile one can, and
|
||||
mode-b `import_bundle` writes the merged concept verbatim. Note the three block-list
|
||||
shapes are *not* one case: flat scalars parse correctly, one key per item misparses
|
||||
silently, two keys per item hard-rejects.
|
||||
- **OKF frontmatter is a restricted grammar: the mapping class has no expressible
|
||||
form.** Gate T2 accepts a line-oriented subset deliberately — full YAML is a larger
|
||||
parse-attack surface than a write-time gate needs. Nested mappings and flow
|
||||
collections (`[a, b]`, `{k: v}`) are *rejected outright*, which fails secure.
|
||||
**All four routes to a mapping fail, each on a different rule** — flow (`{k: v}`)
|
||||
on the disallowed value-start indicator, block (`k:\n sub: v`) on the
|
||||
nested-mapping check, dotted keys (`k.sub: v`) on the key pattern, and the inline
|
||||
second colon (`k: sub: v`) on the mapping-construct check — so the mapping *class*
|
||||
has no expressible form, rather than one form being preferable to another. What
|
||||
survives is scalars and flat lists of strings. **Two of those routes used to
|
||||
degrade into a string instead of failing, and that defect is closed in `1.1.0`**:
|
||||
a block-sequence item carrying exactly one key (`sources:\n - uri: https://e.com/a`
|
||||
yielded the *string* `'uri: https://e.com/a'`) and the inline second colon
|
||||
(`attester: resource: attesters/sql_equality.py`, which a real YAML parser refuses
|
||||
outright). Both parsed "successfully" into the wrong *type*, and a pointer parked
|
||||
in one rode through in a key the `resource` allowlist never inspects — mode-b
|
||||
`import_bundle` returned WARN and wrote the merged concept verbatim. Both now
|
||||
FAIL_SECURE at T2, before the allowlist is reached. **The boundary is where YAML
|
||||
puts it**, ground-truthed against PyYAML 6.0.3: `": "` and a trailing `":"` open a
|
||||
mapping and are refused; a colon carrying neither a space nor a line end
|
||||
(`domain:security`, `https://e.com:8443/a`) does not and still parses, as does a
|
||||
quoted scalar (`- "uri: x"`). Quotes are retained rather than stripped — a
|
||||
divergence from YAML that remains, pinned in `tests/test_okf.py`.
|
||||
- **T2 constrains import, not emission.** The frontmatter grammar runs on
|
||||
`okf.import_bundle` (door C) only — `parse_frontmatter` is referenced nowhere in the
|
||||
door A/B persist path, so frontmatter that fails secure on import passes
|
||||
|
|
|
|||
|
|
@ -64,6 +64,10 @@ _KEY_RE = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9_-]*$")
|
|||
# scalar (|, >), flow collection ([ ] { }), directive (%) or reserved char
|
||||
# (@ `) — all outside the supported subset and all rejected.
|
||||
_DANGEROUS_VALUE_STARTS = frozenset("&*!|>[]{}%@`")
|
||||
# A quoted scalar is a scalar in YAML however many colons it carries, so the
|
||||
# mapping check steps aside for one. The quotes are retained rather than
|
||||
# stripped — a pre-existing divergence, pinned in tests/test_okf.py.
|
||||
_QUOTE_STARTS = frozenset("\"'")
|
||||
|
||||
|
||||
class OKFError(Exception):
|
||||
|
|
@ -576,6 +580,7 @@ def _parse_flat(fm_lines):
|
|||
continue
|
||||
|
||||
_reject_dangerous_value(value)
|
||||
_reject_mapping_construct(value)
|
||||
result[key] = value
|
||||
i += 1
|
||||
|
||||
|
|
@ -601,6 +606,7 @@ def _consume_block_list(fm_lines, start):
|
|||
if raw[:1] in (" ", "\t") and stripped.startswith("- "):
|
||||
item = stripped[2:].strip()
|
||||
_reject_dangerous_value(item)
|
||||
_reject_mapping_construct(item)
|
||||
items.append(item)
|
||||
i += 1
|
||||
continue
|
||||
|
|
@ -616,3 +622,29 @@ def _reject_dangerous_value(value):
|
|||
"value begins with a disallowed YAML indicator %r: %r"
|
||||
% (value[0], value)
|
||||
)
|
||||
|
||||
|
||||
def _reject_mapping_construct(value):
|
||||
"""Reject a scalar that YAML reads as a mapping rather than as a string.
|
||||
|
||||
T2 gives the mapping *class* no expressible form — flow, nested-block and
|
||||
dotted-key routes all raise. Two routes used to escape that by degrading
|
||||
into a string instead: a block-sequence item carrying exactly one key
|
||||
(``- uri: x``), and an inline second colon (``attester: resource: x``).
|
||||
Both parsed "successfully" into the wrong *type*, and a pointer parked in
|
||||
one rode through in a key the ``resource`` allowlist never inspects.
|
||||
|
||||
``": "`` and a trailing ``":"`` are exactly the two shapes where a plain
|
||||
scalar stops being one — ground-truthed against PyYAML 6.0.3, which reads
|
||||
``- uri: x`` as ``[{'uri': 'x'}]``, ``- uri:`` as ``[{'uri': None}]``, and
|
||||
refuses ``k: sub: v`` outright. A colon carrying neither a space nor a line
|
||||
end opens no mapping (``domain:security``, ``https://e.com:8443/a``) and is
|
||||
left alone, as is a quoted scalar — over-blocking a conformant bundle is
|
||||
itself a failure mode.
|
||||
"""
|
||||
if not value or value[0] in _QUOTE_STARTS:
|
||||
return
|
||||
if ": " in value or value.endswith(":"):
|
||||
raise OKFFrontmatterError(
|
||||
"a mapping is not expressible in OKF frontmatter: %r" % (value,)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -577,15 +577,57 @@ def test_v02_flat_frontmatter_still_parses(cid, fm):
|
|||
assert parse_frontmatter(f"---\nid: x\n{fm}---\n\nbody\n")[0]["id"] == "x"
|
||||
|
||||
|
||||
def test_one_key_block_sequence_item_is_misparsed_as_a_string():
|
||||
# The documented defect: two keys per item hard-reject (loud, safe), but ONE key
|
||||
# parses "successfully" into the wrong type. A consumer reading
|
||||
# frontmatter["sources"][0].get("uri") gets a string, not a mapping.
|
||||
fm, _ = parse_frontmatter(
|
||||
"---\nid: x\nsources:\n - uri: https://e.com/a\n---\n\nbody\n"
|
||||
)
|
||||
assert fm["sources"] == ["uri: https://e.com/a"], "shape changed — update LIMITATIONS.md"
|
||||
assert not isinstance(fm["sources"][0], dict)
|
||||
# --- the type-confusion defect, closed in 1.1.0 (2026-08-13) ----------------
|
||||
# Was: a mapping construct that the restricted grammar cannot represent degraded
|
||||
# into a STRING instead of failing. Two routes did this, not the one documented.
|
||||
# Ground-truthed against PyYAML 6.0.3: every shape below that we now reject is a
|
||||
# shape a real YAML parser reads as a MAPPING (or refuses outright), and every
|
||||
# shape we still admit is one PyYAML reads as a plain scalar.
|
||||
|
||||
_DEGRADED_TO_STRING = [
|
||||
# (id, frontmatter, what PyYAML 6.0.3 makes of it)
|
||||
("one key per item", "sources:\n - uri: https://e.com/a\n", "[{'uri': ...}]"),
|
||||
("item, trailing colon", "sources:\n - uri:\n", "[{'uri': None}]"),
|
||||
("inline double colon", "attester: resource: attesters/sql_equality.py\n", "parse error"),
|
||||
("top value, trailing colon", "description: see below:\n", "parse error"),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("cid,fm,yaml_reads_as", _DEGRADED_TO_STRING,
|
||||
ids=[c[0] for c in _DEGRADED_TO_STRING])
|
||||
def test_a_mapping_construct_never_degrades_into_a_string(cid, fm, yaml_reads_as):
|
||||
# The mapping *class* has no expressible form through T2 — so a mapping
|
||||
# construct must RAISE, never parse "successfully" into the wrong type. A
|
||||
# consumer reading frontmatter["sources"][0].get("uri") must not be handed a str.
|
||||
with pytest.raises(OKFFrontmatterError):
|
||||
parse_frontmatter(f"---\nid: x\n{fm}---\n\nbody\n")
|
||||
|
||||
|
||||
_STILL_SCALARS = [
|
||||
# PyYAML reads every one of these as a plain scalar: the colon carries no
|
||||
# space and no line end, so it never opens a mapping. Over-blocking a
|
||||
# conformant bundle is itself a failure mode (brief principle 5).
|
||||
("colon, no space", "tags:\n - domain:security\n", "tags", ["domain:security"]),
|
||||
("url item", "sources:\n - https://e.com/a\n", "sources", ["https://e.com/a"]),
|
||||
("url item with port", "sources:\n - https://e.com:8443/a\n", "sources",
|
||||
["https://e.com:8443/a"]),
|
||||
("url value with port", "resource: https://e.com:8443/a\n", "resource",
|
||||
"https://e.com:8443/a"),
|
||||
("double-quoted item", 'sources:\n - "uri: https://e.com/a"\n', "sources",
|
||||
['"uri: https://e.com/a"']),
|
||||
("single-quoted item", "sources:\n - 'uri: https://e.com/a'\n", "sources",
|
||||
["'uri: https://e.com/a'"]),
|
||||
("quoted top value", 'description: "Note: careful"\n', "description",
|
||||
'"Note: careful"'),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("cid,fm,key,expected", _STILL_SCALARS,
|
||||
ids=[c[0] for c in _STILL_SCALARS])
|
||||
def test_scalars_that_merely_contain_a_colon_still_parse(cid, fm, key, expected):
|
||||
# Quotes are retained rather than stripped — a pre-existing divergence from
|
||||
# YAML, pinned here so closing the mapping hole is not read as fixing it.
|
||||
assert parse_frontmatter(f"---\nid: x\n{fm}---\n\nbody\n")[0][key] == expected
|
||||
|
||||
|
||||
def test_relative_resource_pointer_fails_the_allowlist():
|
||||
|
|
@ -595,15 +637,18 @@ def test_relative_resource_pointer_fails_the_allowlist():
|
|||
validate_resource_url(pointer)
|
||||
|
||||
|
||||
def test_pointer_in_one_key_sequence_reaches_the_consumer_tree():
|
||||
# The security-relevant consequence of the misparse above: the pointer never
|
||||
# touches the top-level `resource` key, so the https allowlist never inspects it
|
||||
# and door C admits the concept. Not conformant OKF — a well-formed bundle will
|
||||
# not produce this shape — but mode-b writes the merged concept verbatim.
|
||||
doc = ("---\nid: x\ntype: Attested Computation\n"
|
||||
"attester:\n - resource: attesters/sql_equality.py\n---\n\nbody\n")
|
||||
@pytest.mark.parametrize("cid,carrier", [
|
||||
("block sequence", "attester:\n - resource: attesters/sql_equality.py\n"),
|
||||
("inline double colon", "attester: resource: attesters/sql_equality.py\n"),
|
||||
])
|
||||
def test_pointer_in_a_degraded_mapping_no_longer_reaches_the_consumer_tree(cid, carrier):
|
||||
# The security-relevant consequence, closed at door C. Both carriers put the
|
||||
# pointer in a key the https allowlist never inspects, so while the shape
|
||||
# parsed, mode-b wrote the merged concept verbatim. It now fails secure at T2,
|
||||
# before the allowlist is even reached.
|
||||
doc = f"---\nid: x\ntype: Attested Computation\n{carrier}---\n\nbody\n"
|
||||
result = import_bundle({"computations/x.md": doc})
|
||||
assert result.disposition is Disposition.WARN, "hole closed — update LIMITATIONS.md"
|
||||
assert result.disposition is Disposition.FAIL_SECURE, "hole reopened — see LIMITATIONS.md"
|
||||
|
||||
|
||||
def test_every_route_to_a_mapping_fails_on_a_different_rule():
|
||||
|
|
@ -615,6 +660,7 @@ def test_every_route_to_a_mapping_fails_on_a_different_rule():
|
|||
"flow": "generated: { by: x, at: y }\n",
|
||||
"block": "generated:\n by: x\n",
|
||||
"dotted": "generated.by: x\n",
|
||||
"inline": "generated: by: x\n",
|
||||
}
|
||||
errors = {}
|
||||
for name, fm in routes.items():
|
||||
|
|
@ -624,13 +670,15 @@ def test_every_route_to_a_mapping_fails_on_a_different_rule():
|
|||
assert "indicator" in errors["flow"]
|
||||
assert "nested mappings" in errors["block"]
|
||||
assert "key" in errors["dotted"]
|
||||
assert len(set(errors.values())) == 3, "routes must fail distinctly, not collapse"
|
||||
assert "mapping" in errors["inline"]
|
||||
assert len(set(errors.values())) == 4, "routes must fail distinctly, not collapse"
|
||||
|
||||
|
||||
_BLOCK_LIST_ITEM_SHAPES = [
|
||||
# A consumer called all three "the sources block list"; the parser does not.
|
||||
# The one-key-per-item row lived here until 1.1.0, admitted as the string
|
||||
# "id: a"; it now hard-rejects with the two-key row (_DEGRADED_TO_STRING).
|
||||
("flat scalars", "sources:\n - file://x\n - file://y\n", ["file://x", "file://y"]),
|
||||
("one key per item", "sources:\n - id: a\n", ["id: a"]), # silent misparse
|
||||
("single-element", "verified:\n - human:ktg\n", ["human:ktg"]),
|
||||
]
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue