feat: upgrade cookie dependency and cleanup imports

siimsams commented 1 month ago

What kind of change does this PR introduce?

Bump cookie package to the latest version.
Use correct types from cookie package.
Fix typescript warning.
Cleanup unused imports.

What is the current behavior?

It currently shows up as unfixable security issue in my project. https://github.com/advisories/GHSA-pxg6-pf52-xh8x

What is the new behavior?

The new version of this package does not have this security issue.

Additional context

siimsams commented 1 month ago

@alaister @dshukertjr

Wdyt? Could this be merged? Then I could start to look at another issue with the latest next.js.

EDIT: I fixed the lint. Forgot to run it.

siimsams commented 1 month ago

@alaister @dshukertjr

Fixed the lint problems.

retr0cube commented 1 month ago

@alaister @dshukertjr

Wdyt? Could this be merged? Then I could start to look at another issue with the latest next.js.

EDIT: I fixed the lint. Forgot to run it.

This issue is also happening to me with SvelteKit:

cookie  <0.7.0
cookie accepts cookie name, path, and domain with out of bounds characters - https://github.com/advisories/GHSA-pxg6-pf52-xh8x
fix available via `npm audit fix --force`
Will install @sveltejs/kit@0.0.30, which is a breaking change
node_modules/cookie
  @supabase/ssr  *
  Depends on vulnerable versions of cookie
  node_modules/@supabase/ssr
  @sveltejs/kit  >=1.0.0-next.0
  Depends on vulnerable versions of cookie
  node_modules/@sveltejs/kit
    @sveltejs/adapter-auto  >=1.0.0-next.0
    Depends on vulnerable versions of @sveltejs/kit
    node_modules/@sveltejs/adapter-auto
    @sveltejs/adapter-vercel  >=1.0.0-next.0
    Depends on vulnerable versions of @sveltejs/kit
    node_modules/@sveltejs/adapter-vercel

5 low severity vulnerabilities

To address all issues possible (including breaking changes), run:
  npm audit fix --force

Some issues need review, and may require choosing
a different dependency.

siimsams commented 1 month ago

@J0 Thank you for the review. Do I need to do anything additional for this to be merged and released?

J0 commented 1 month ago

Could you regenerate the package-lock.json and package.json? Apologies I was slightly hesitant to bump the version to v1.0.1 as that's a jump in major version. I went ahead and bumped the minor version which resulted in some conflicts. The minor version bump to v0.7.0 should also resolve the warning for now I believe

We'll still consider the v1.0.1 upgrade and changes but I need to check in with the team before I go ahead and merge.

siimsams commented 1 month ago

@J0 Thank you for the feedback. I regenerated the package-lock.json.

The breaking changes are described here: https://github.com/jshttp/cookie/releases

siimsams commented 1 month ago

@J0 The security fix has not been released as of now. It's just a RC version not a published version.

I get that this needs some more validation but can you guys release the current RC version as V5.0.2? Then we could release this as 6.0.0 and list the minimum node version as breaking change.

lorikku commented 3 weeks ago

When will this be merged? Want to fix my audits :)

J0 commented 3 weeks ago

Hey apologies for missing this.

We are releasing v0.5.2 now.

J0 commented 3 weeks ago

Thanks for your patience @siimsams

AFAICT this shouldn't affect our API beyond the requirement for an increment in node version to v18 (current LTS is v20)

I think it should be fine to merge this as a minor version bump so going to merge. Welcome dissenting opinions though.

This should live in rc for a while, which will give us time to test.

siimsams commented 2 weeks ago

Thank you for releasing the fix. Not in a hurry with this PR.

supabase / ssr