tegwick
ebc48e31ae
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NV9oijZukGyGbRQGGKnK4P Assistant: claude-code Assistant-Model: opus Assistant-Process: 713576@bnt-lap001 Assistant-Session: 384c511d-9bce-4cb8-a676-2aef6c0c8df6
101 lines
4.3 KiB
Markdown
101 lines
4.3 KiB
Markdown
---
|
|
id: KEY-WP-0021
|
|
type: workplan
|
|
title: "Make snapshot attribute validation enforce a real rule"
|
|
domain: infotech
|
|
repo: key-cape
|
|
status: finished
|
|
owner: claude
|
|
topic_slug: snapshot-attribute-validation
|
|
created: "2026-09-07"
|
|
updated: "2026-09-07"
|
|
state_hub_workstream_id: "733ad1a5-c4d8-5c2f-8608-c51bf1d5b17e"
|
|
---
|
|
|
|
Closes gap G06 of `history/2026-09-05-011726-scope-intent-assessment.md`:
|
|
`checkNoUnknownAttributes` is named for a rule it does not implement. It rejects
|
|
blank keys, which is not an allow-list, so a snapshot carrying arbitrary or
|
|
malformed LDAP attribute names passes a check whose name says otherwise. A rule
|
|
that passes for the wrong reason is worse than an absent one, because a report
|
|
listing `no_unknown_attributes: passed` is read as evidence.
|
|
|
|
One part of the assessment's description needs correcting rather than
|
|
implementing: it says group membership validation checks nonempty member IDs
|
|
rather than complete referenced identity existence. `checkValidGroupMemberships`
|
|
does only check emptiness, but the semantic rule `checkReferencedUsersExist`
|
|
already resolves every member against the user set and fails on an unknown one.
|
|
The reference constraint exists; it lives in a different rule than the one the
|
|
assessment looked at.
|
|
|
|
## Enforce real attribute constraints
|
|
|
|
```task
|
|
id: KEY-WP-0021-T01
|
|
status: done
|
|
priority: medium
|
|
state_hub_task_id: "e2921983-853c-501b-8dfc-158751dea093"
|
|
```
|
|
|
|
`ldapAttributes` is defined by the canonical model as raw attributes *not covered
|
|
by* the model, so an allow-list of permitted names cannot be derived from the
|
|
schema — the field exists precisely to carry what the schema does not name. Two
|
|
constraints are derivable and worth enforcing:
|
|
|
|
- the attribute name must be a syntactically valid LDAP attribute descriptor
|
|
(RFC 4512: a letter followed by letters, digits or hyphens, or a numeric OID),
|
|
since a name that cannot be written to a directory cannot round-trip; and
|
|
- it must not shadow a canonical field's documented LDAP mapping (`uid`, `cn`,
|
|
`mail`), because two sources for one value is how an export and a directory
|
|
come to disagree, and the model already owns those.
|
|
|
|
Attribute descriptors are case-insensitive in LDAP, so keys differing only by
|
|
case are a collision, not two attributes. Keep the blank-key check. Rename the
|
|
rule to describe what it actually enforces rather than leaving a name that
|
|
overpromises.
|
|
|
|
Implemented as `raw_attributes_well_formed`, replacing `no_unknown_attributes`
|
|
in both `validator.go` and `spec/ldap-schema.yaml`, with the schema entry stating
|
|
plainly that this is not an allow-list and why one cannot be derived.
|
|
|
|
## Prove each rule with an invalid snapshot
|
|
|
|
```task
|
|
id: KEY-WP-0021-T02
|
|
status: done
|
|
priority: medium
|
|
state_hub_task_id: "de4caf0a-39c6-5148-a986-db18a1308505"
|
|
```
|
|
|
|
The assessment asks for invalid-snapshot cases proving each stated rule. Add one
|
|
per constraint — malformed descriptor, shadowed canonical mapping, case-only
|
|
duplicate, blank key — plus passing cases for legitimate raw attributes and a
|
|
valid numeric OID, so the rule is shown to be selective rather than merely
|
|
strict.
|
|
|
|
Eleven malformed-descriptor cases, six canonical-mapping cases across casings, a
|
|
case-only duplicate, a message-content case, and five accepted-attribute cases.
|
|
Confirmed they test the rule rather than merely passing: making the check return
|
|
early fails four of the five tests.
|
|
|
|
## Reconcile the records
|
|
|
|
```task
|
|
id: KEY-WP-0021-T03
|
|
status: done
|
|
priority: medium
|
|
state_hub_task_id: "8a48c6cc-1e7e-5fd4-abfa-2b893c3d400d"
|
|
```
|
|
|
|
Update `SCOPE.md` and G06's status. State what the rule now enforces and, just as
|
|
importantly, what it still does not: this is not schema-derived validation of
|
|
every attribute against a directory schema, and the canonical model cannot
|
|
provide one for a field defined as everything the model does not cover. Correct
|
|
the assessment's reference-constraint premise rather than restating it.
|
|
|
|
`history/…-scope-intent-assessment.md` was being edited by a concurrent session
|
|
when this workplan was written; make this edit only against a clean tree.
|
|
|
|
Done against a clean tree after the other session committed. G06's status records
|
|
both corrections: no allow-list is derivable for a field defined as what the
|
|
model does not cover, and the reference constraint was already implemented in
|
|
`checkReferencedUsersExist` rather than missing.
|