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

fix: don't allow . or .. in feature url #8479

Merged

Conversation

Tymek
Copy link
Member

@Tymek Tymek commented Oct 18, 2024

We do some validation on flag names, but there's some cases that slip through. These are some cases that we should handle better.

With .. as a name, you can't go into the flag in Unleash and you can't activate any environments because the it is interpreted as "go up a level".

Copy link

vercel bot commented Oct 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Oct 21, 2024 2:01pm
unleash-monorepo-frontend ⬜️ Ignored (Inspect) Visit Preview Oct 21, 2024 2:01pm

Copy link
Contributor

github-actions bot commented Oct 18, 2024

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change applies to features, tags, projects, context fields and addons

@Tymek Tymek changed the title 1 2986 some invalid flag names arent caught by validation eg fix: don't allow . or .. in feature url Oct 18, 2024
Comment on lines +17 to +19
encodeURIComponent(value) !== value ||
value === '..' ||
value === '.'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh, this is good. Are there other cases we might have missed?

I checked the docs for this function, and there's also - _ ! ~ * ' ( ):

image

Do we want to allow them? I guess there's no harm in it because they don't break urls the same way. Sure, let's go.

@Tymek Tymek force-pushed the 1-2986-some-invalid-flag-names-arent-caught-by-validation-eg branch from 639818b to 24789d2 Compare October 21, 2024 13:59
@Tymek Tymek enabled auto-merge (squash) October 21, 2024 14:12
@Tymek Tymek requested a review from thomasheartman October 21, 2024 14:12
@Tymek Tymek merged commit 2e970b0 into main Oct 21, 2024
7 checks passed
@Tymek Tymek deleted the 1-2986-some-invalid-flag-names-arent-caught-by-validation-eg branch October 21, 2024 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants