Skip to content

feat: add icon and description fields to workspace preset #422

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

Merged
merged 7 commits into from
Jul 22, 2025

Conversation

ssncferreira
Copy link
Contributor

@ssncferreira ssncferreira commented Jul 21, 2025

Description:

This PR adds two new optional fields to the coder_workspace_preset Terraform data source:

  • icon: A URL string pointing to an icon to display in the dashboard.
  • description: A text field describing the purpose of the preset.

These fields help improve the user experience by allowing presets to be visually distinguished and better documented.

Changes:

  • Added icon and description to the Terraform schema for coder_workspace_preset.
  • Included these fields in the test case TestWorkspacePreset.
  • Updated the integration tests accordingly.

Related to: coder/coder#18111

@@ -93,6 +96,29 @@ func workspacePresetDataSource() *schema.Resource {
Required: true,
ValidateFunc: validation.StringIsNotEmpty,
},
"description": {
Copy link
Contributor Author

@ssncferreira ssncferreira Jul 21, 2025

Choose a reason for hiding this comment

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

Copied the schema definition for description and icon from the parameter schema (see https://github.com/coder/terraform-provider-coder/blob/main/provider/parameter.go#L184). Neither field currently has a size limit, but it might be good to add one to ensure the UI isn’t broken by overly large descriptions. Wdyt?

There is also a security consideration: we don’t seem to perform proper escaping or sanitization, which means it could be possible to inject HTML or JavaScript via the description. For the icon, since it’s used to fetch local files, we should be careful to prevent directory traversal attacks or other malicious paths.

Given that Coder typically runs on-premises, these risks might be lower, but it could still be a good practice to address them. Do we currently have any mechanisms in place to handle these concerns?

Copy link
Member

Choose a reason for hiding this comment

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

Neither field currently has a size limit

Do you mean in the provider, or in the database?

For the icon, since it’s used to fetch local files

What do you mean by 'local' here? The icon is used by the UI, and the icons are generally hosted on the control plane, although I've seen people link to icons on other domains.

There is also a security consideration: we don’t seem to perform proper escaping or sanitization

Where specifically have you checked?

Do we currently have any mechanisms in place to handle these concerns?

Template admins have a good bit of power in a Coder deployment, so there is some element of trust related to allowing users to create templates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean in the provider, or in the database?

In both:

I think it would make sense to add reasonable size limits to both description and icon, similar to what we do for templates table:

Wdyt?

What do you mean by 'local' here? The icon is used by the UI, and the icons are generally hosted on the control plane, although I've seen people link to icons on other domains.

By "local" I meant relative paths like /icon/region.svg. Given that, someone could technically enter a path like ../../etc/passwd or something similar.

Where specifically have you checked?

I tested the integration between the Terraform provider and Coder. I inserted a description with inline JavaScript and confirmed it was passed through and stored in the database. However, from my quick test, the UI seems to display the content as plain text and doesn’t interpret or render it as raw HTML, so that is good. I didn't test the icon path.

Template admins have a good bit of power in a Coder deployment, so there is some element of trust related to allowing users to create templates.

Totally agree. Given Coder is self-hosted and template creation is an admin-level task, I don't think this is a critical issue, more of an observation from working on this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would make sense to add reasonable size limits to both description and icon, similar to what we do for templates table

I think that's reasonable!

By "local" I meant relative paths like /icon/region.svg. Given that, someone could technically enter a path like ../../etc/passwd or something similar.

Right, but I don't think we would expose that in our static file server though. Might be no harm to sanitize the path though, good callout!

Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

non-blocking: ideally we should include an example in examples dir

Comment on lines +23 to +25
name = "preset"
description = "preset description"
icon = "preset icon"
Copy link
Member

Choose a reason for hiding this comment

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

nit: should we use rather realistic values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed the naming convention used by the other parameters, as they follow a similar logic. Since this is an integration test, I don’t think adding realistic values here is necessary.

@ssncferreira
Copy link
Contributor Author

non-blocking: ideally we should include an example in examples dir

I was not aware of this examples dir. Addressed in d2efd30

@ssncferreira ssncferreira merged commit e6d58d0 into main Jul 22, 2025
6 checks passed
@ssncferreira ssncferreira deleted the ssncferreira/feat-preset-icon-description branch July 22, 2025 17:11
@github-actions github-actions bot locked and limited conversation to collaborators Jul 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants