Skip to content

Reject a non-UUID guid before it becomes a file id#304

Merged
kptdobe merged 2 commits into
mainfrom
da-versions-source-guid-validation
Jul 17, 2026
Merged

Reject a non-UUID guid before it becomes a file id#304
kptdobe merged 2 commits into
mainfrom
da-versions-source-guid-validation

Conversation

@benpeter

Copy link
Copy Markdown
Contributor

What

A source PUT or POST can send a guid form field. When the object has no stored
file id, that guid becomes the file id. The file id is a path segment in the
reserved .da-versions key space.

The old code took the guid as is. A slash, a .da-versions segment, or any
non-UUID value became the file id and shaped the keys. Now the guid must be a
plain UUID. A file id read back from stored metadata is also untrusted. An unsafe
stored id no longer builds reserved keys.

Change

  • Add isValidId to src/storage/version/paths.js. It allowlists the UUID
    shape. It returns true only for a plain UUID.
  • In putObjectWithVersion, reject a supplied guid that fails the allowlist
    with a 400. This runs before the guid becomes the file id.
  • Add isSafeId to src/storage/version/paths.js. Apply it to the file id from
    stored metadata. Stored metadata is untrusted. An id written before this check
    could carry key steering characters. The storage transport turns the key into a
    URL. That URL collapses a dot or double dot segment, treats a backslash as a
    separator, decodes a percent escape, and strips whitespace. isSafeId refuses
    an id that uses any of those. It also refuses a slash or a .da-versions
    segment. It is weaker than the UUID allowlist on purpose. A benign legacy
    single-segment id still works.
  • Document the optional guid field, its UUID constraint, the 400 response, and
    the 409 mismatch response on the source API (docs/openapi/schemas.yaml,
    docs/openapi/source-api.yaml). Regenerate docs/index.html.

A missing or empty guid still falls back to a generated UUID. Normal writes do
not change. A document with a benign stored id still works.

Testing

  • Unit tests cover the rejected guid shapes: slash, leading slash,
    .da-versions segment, non-UUID string, and a UUID with a trailing path. A
    valid UUID is still accepted and stored as the document id.
  • Unit tests cover isValidId and isSafeId directly. A regression suite
    refuses an unsafe stored id (slash, .da-versions segment, bare .da-versions,
    a single or double dot segment, a backslash, and a percent-encoded dot segment)
    with a 400 and writes nothing. A benign legacy single-segment stored id,
    including one that contains a dot, still updates.
  • Integration tests confirm a source PUT with a crafted guid returns 400. A
    source PUT with a valid UUID guid returns 201 with the id in the X-da-id
    header.
  • An OpenAPI contract test asserts the source schema and the PUT and POST
    descriptions keep documenting the guid field, the UUID constraint, and the
    400 and 409 responses.
  • npm test, npm run lint, and npm run test:it all pass.

benpeter added 2 commits July 16, 2026 15:55
…le id

A source PUT can supply a guid form field. On a new document that guid
becomes the file id and is written into the reserved .da-versions key
space. These tests assert that a guid which is not a plain UUID, or which
contains a slash or a .da-versions segment, is rejected with 400 and never
written to storage, while a valid UUID guid still works. The tests are red
against current code.
…ile id

A source PUT or POST may include a guid form field. When the target has no
stored file id yet, that guid becomes the document file id. The file id is
then used as a path segment inside the reserved .da-versions key space.

Before this change the guid was accepted without any checks. A value with a
slash or a .da-versions segment, or any other value that is not a plain UUID,
was stored as the file id and used to build keys. This change validates the
guid against a UUID allowlist so only a plain UUID is accepted as a new file
id. A missing or empty guid still falls back to a generated UUID, so normal
writes are unaffected.

A file id read back from stored metadata is also treated as untrusted. An id
persisted before this validation existed could contain key steering
characters. The storage transport builds a URL from the key, so it collapses a
dot or double dot segment, treats a backslash as a separator, decodes a percent
escape, and strips whitespace. A stored id that uses any of those, or that has
a slash or a .da-versions segment, is now refused before it is used to build
reserved keys. This check is weaker than the UUID allowlist on purpose, so a
benign legacy single-segment id keeps working.

Document the optional guid field, its UUID constraint, the 400 response, and
the 409 response for a mismatch against an existing stored identifier on the
source API, and regenerate the API docs.

Add unit, integration, and OpenAPI contract tests for the new validation.
@benpeter
benpeter marked this pull request as ready for review July 16, 2026 18:36
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@kptdobe
kptdobe merged commit 818c553 into main Jul 17, 2026
6 checks passed
@kptdobe
kptdobe deleted the da-versions-source-guid-validation branch July 17, 2026 08:34
adobe-bot pushed a commit that referenced this pull request Jul 17, 2026
# [1.13.0](v1.12.4...v1.13.0) (2026-07-17)

### Features

* reject a non-UUID guid before it becomes a file id ([#304](#304)) ([818c553](818c553))
@adobe-bot

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.13.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants