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

[Security] Make release URL as constant type (fix CWE-88) #141

Closed
wants to merge 34 commits into from

Conversation

alphaX86
Copy link
Member

@alphaX86 alphaX86 commented Jan 1, 2022

Signed-off-by: Aadhitya A [email protected]

Description

This PR fixes #138

Notes for Reviewers
Gosec checks need to be monitored

Signed commits

  • Yes, I signed my commits.

@welcome
Copy link

welcome bot commented Jan 1, 2022

Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, please review the Layer5 Community Welcome Guide and sure to join the community Slack.
Be sure to double-check that you have signed your commits. Here are instructions for making signing an implicit activity while peforming a commit.

@alphaX86 alphaX86 requested a review from a team January 1, 2022 14:41
releaseAPIURL := "https://api.github.com/repos/traefik/mesh/releases?per_page=" + fmt.Sprint(releases)
func GetLatestReleases() ([]*Release, error) {
// Making the results to 10 to avoid fetching lot of releases (to avoid CWE-88)
const releaseAPIURL = "https://api.github.com/repos/traefik/mesh/releases?per_page=10"
Copy link
Member

Choose a reason for hiding this comment

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

Makes sense.

@meshery/adapter-maintainers please take note.

Copy link
Contributor

Choose a reason for hiding this comment

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

This uses github API, I have added the page scraping way in meshkit. I think, that should be reused everywhere. We need to get off github API completely for fetching latest releases

Copy link
Member

Choose a reason for hiding this comment

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

Yes, very good point, @Revolyssup.

@manav1403 or @piyushsingariya might point out examples of where this has been done elsewhere, so that we can move entirely away from any api.github.com requests and over to github.com requests.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

OK... This makes sense, if that change is implemented I'll include the method here too

Signed-off-by: Unnati <[email protected]>
Signed-off-by: Unnati <[email protected]>
@s1ntaxe770r
Copy link

@alphaX86 thanks for submitting a PR , security fixes are always welcom

leecalcote and others added 22 commits January 5, 2022 13:09
Updated slack.yml and added newcomers.yml to workflows folder
Signed-off-by: Pranav Singh <[email protected]>
Signed-off-by: Pranav Singh <[email protected]>
[Cl] Fix minor syntax error in e2etest.yaml
[Cl] Add step for generating short_sha for the filename
Add Ashish and Rudraksh in reviewers
Signed-off-by: ashish <[email protected]>
Signed-off-by: ashish <[email protected]>
Signed-off-by: ashish <[email protected]>
Add build time and modify run time comp generation
dependabot bot and others added 9 commits January 24, 2022 20:03
Bumps [github.com/layer5io/meshkit](https://github.com/layer5io/meshkit) from 0.2.34 to 0.5.2.
- [Release notes](https://github.com/layer5io/meshkit/releases)
- [Commits](meshery/meshkit@v0.2.34...v0.5.2)

---
updated-dependencies:
- dependency-name: github.com/layer5io/meshkit
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…hub.com/layer5io/meshkit-0.5.2

Bump github.com/layer5io/meshkit from 0.2.34 to 0.5.2
Signed-off-by: ashish <[email protected]>
@leecalcote
Copy link
Member

@alphaX86, it seems that this branch might need resynced.

@alphaX86
Copy link
Member Author

@alphaX86, it seems that this branch might need resynced.

Noted that Lee 👍 I may need to rebase the branch correctly and implement the function which @Revolyssup has created in Meshkit to solve this up

@alphaX86
Copy link
Member Author

I'll open a new PR... in mean time I'll close this one as it's bit dirty due to improper rebasing

@alphaX86 alphaX86 closed this Feb 20, 2022
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.

[Security] (CWE-88): Potential HTTP request made with variable url
8 participants