Skip to content

fix: keep authentic signed attributes when an extended attribute collides - #1370

Open
arpitjain099 wants to merge 1 commit into
notaryproject:mainfrom
arpitjain099:fix/inspect-signed-attribute-collision
Open

arpitjain099 wants to merge 1 commit into
notaryproject:mainfrom
arpitjain099:fix/inspect-signed-attribute-collision

Conversation

@arpitjain099

Copy link
Copy Markdown

Description

getSignedAttributes seeds a flat map with contentType, signingScheme, signingTime and expiry, then writes every envelope-supplied extended attribute into the same map. An extended attribute keyed on one of those four names therefore replaces the authentic signed value, and the real one never reaches notation inspect -o json or notation blob inspect -o json. The tree handler is not affected, because it uses spaced display labels and AddPair appends, so it emits both rows.

The fix is to populate the extended attributes first and write the four well-known keys after, so the authentic value always wins. The flat schema is documented in specs/cmd/inspect.md, so I kept it rather than namespacing anything.

Worth saying what this is not: a signer can already set SigningTime and Expiry to whatever they like in the SignRequest, and inspect performs no verification, so nothing trusts this output. It is data loss in a machine-readable view, where two distinct fields collapse into one.

TestGetSignedAttributesCollision builds an envelope with a colliding attribute for each of the four keys plus one benign attribute. On main all four assertions fail, for instance expected signingTime 2023-11-14 17:13:20 -0500 EST, but got 1970-01-01T00:00:00Z; the benign attribute passes either way. Existing TestGetSignedAttributes is untouched and go test ./... is green.

An alternative would be to skip the colliding key and log it instead of dropping it silently. Happy to switch if you would rather surface the collision.

AI assistance was used on this change and is disclosed in the commit trailer.

…ides

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant