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

Prevent cluster groups from being deleted when referenced by a projects' configuration #15119

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

markylaing
Copy link
Contributor

Adds an API extension clustering_groups_used_by and adds a UsedBy field to api.ClusterGroup. On GET requests, the UsedBy URLs are projects that contain the cluster group in restricted.cluster.groups. DELETE requests will fail if the cluster group is used by a project.

Closes #15118

Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
This duplicates `(*cluster.ClusterGroup).ToAPI`.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
This function queries for projects that have the group name
present in the value for the config key `restricted.cluster.groups`.
Then it parses the config value properly to check for an exact match.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
This includes a small refactor of `GET /1.0/cluster/groups`
where the handler was unnecessarily iterating over the list of
groups twice.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
…ced by a project.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing markylaing added the Bug Confirmed to be a bug label Mar 6, 2025
@markylaing markylaing self-assigned this Mar 6, 2025
@github-actions github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Mar 6, 2025
JOIN projects_config ON projects.id = projects_config.project_id
WHERE projects_config.key = 'restricted.cluster.groups' AND projects_config.value LIKE '%` + groupName + `%'`

stmt, err := tx.Prepare(q)

Check failure

Code scanning / CodeQL

Database query built from user-controlled sources High

This query depends on a
user-provided value
.
q := `
SELECT projects.name, projects_config.value FROM projects
JOIN projects_config ON projects.id = projects_config.project_id
WHERE projects_config.key = 'restricted.cluster.groups' AND projects_config.value LIKE '%` + groupName + `%'`
Copy link
Member

Choose a reason for hiding this comment

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

This is a SQL injection security risk.

Copy link
Member

Choose a reason for hiding this comment

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

Also the use of LIKE here seems potentially incorrect as it might match partial group names.

Can we use JSON collation and direct matching with query parameters instead?

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Looks good, except the SQL injection flaw ;)

@markylaing
Copy link
Contributor Author

Looks good, except the SQL injection flaw ;)

Whoopsie 😆 Thank you CodeQL 🙏 Not sure how I convinced myself that this was ok...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes to the REST API Bug Confirmed to be a bug Documentation Documentation needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

project config 'restricted.cluster.groups' not checked when deleting a cluster group
2 participants