Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

slug defaultIsUnique failing when content releases do not exist - allowing duplicate slugs in live systems #8724

Open
ChrisNolan opened this issue Feb 22, 2025 · 2 comments

Comments

@ChrisNolan
Copy link
Contributor

If you find a security vulnerability, do NOT open an issue. Email [email protected] instead.

Describe the bug

Slugs are being allowed to be non-unique by default now, due to the new 'versions' system (PR #8454 ). I feel this is serious as any system which has had invalid slugs introduced since the change will have to retroactively go back and update their data. (Sorry couldn't find which of the commits in that PR did the deed).

I believe the issue is this line added to the constraints in the slugValidator to check if the document id exists in the content releases... but if you have no content releases we have a problem.
!(_id in path("${VERSION_FOLDER}.**.${published}")),

To Reproduce

Steps to reproduce the behavior:

update a document with a basic slug field to use a slug field that is already in use on another document with content releases not enabled. No validation error is given.

Note, using the browser tools I could see for example the query being made on the validator is

!defined(*[
  _type == "contributor" &&
  !(_id in ["drafts.chris-nolan-1978", "chris-nolan-1978"]) &&
  !(_id in path("versions.**.chris-nolan-1978")) &&
  slug.current == "chris-nolan"
][0]._id)

and this returns 'true'. Where the query before the change from the 'content releases'

!defined(*[
  _type == "contributor" &&
  !(_id in ["drafts.chris-nolan-1978", "chris-nolan-1978"]) &&
  slug.current == "chris-nolan"
][0]._id)

returns false -- as there is another 'contributor' with the slug of 'chris-nolan'.

So the issue is that "all of them" have to be true to work... and if we have no versions at all, it is failing. The constraints in slugValidator should be something like

const constraints = [
  '_type == $docType',
  `!( 
    _id == $published || 
    _id == $draft || 
    _id in path("${VERSION_FOLDER}.**.${published}")
  )`,
  `${atPath} == $slug`,
].join(' && ');

instead. Good ol logical operators running afoul.

Likely needs to be tested with content releases enabled, and content releases not enabled. Maybe the other fix could be to only add that element to the constraints array if content releases are enabled?

Expected behavior

A validation error should be given.

Also, why are the tests in infer.ts still passing...? Ah... because they were changed to expect a mock of a query that matches this form, they aren't actually doing the query and seeing the failed results.

Screenshots
If applicable, add screenshots to help explain your problem.

Which versions of Sanity are you using?

Run sanity versions in the terminal and copy-paste the result here.

What operating system are you using?

Which versions of Node.js / npm are you running?

Run npm -v && node -v in the terminal and copy-paste the result here.

Additional context

See this thread on the community slack.

@ChrisNolan ChrisNolan changed the title slug defaultIsUnique failing due to 'versions' - allowing duplicate slugs in live systems slug defaultIsUnique failing when content releases do not exist - allowing duplicate slugs in live systems Feb 22, 2025
@ChrisNolan
Copy link
Contributor Author

@pedrobonamin I believe 'content releases' are your 'baby'? Would you be able to take a look at this and perhaps collaborate on the #8725 PR as I'm not sure how to actually run test studios and things?

@ChrisNolan
Copy link
Contributor Author

Also, I was only looking at the issue with the slug validator, but likely anywhere which had code updated to also compare versions.** along with drafts and published need to be double checked too. Maybe that could be put into a 'fragment' and re-used perhaps?

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

No branches or pull requests

1 participant