Skip to content

Conversation

@Foorack
Copy link

@Foorack Foorack commented Sep 27, 2025

Description

This PR adds support for configuring trusted domains in the code-server module through a new trusted_domains variable.

This allows users to specify domains that should be trusted for link protection using code-server's --link-protection-trusted-domains option.

Type of Change

  • New module
  • Bug fix
  • Feature/enhancement
  • Documentation
  • Other

Module Information

Path: registry/coder/modules/code-server
New version: N/A
Breaking change: [ ] Yes [X] No

Related Issues

microsoft/vscode#82794

coder/coder#19995

@Copilot Copilot AI review requested due to automatic review settings September 27, 2025 16:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for configuring trusted domains in the code-server module to enable link protection functionality. Users can now specify a list of domains that should be trusted when code-server validates external links.

  • Adds a new trusted_domains variable to accept a list of trusted domain strings
  • Updates the run script to process the domains and pass them to code-server via --link-protection-trusted-domains flags
  • Integrates the trusted domains configuration into the coder_script resource environment

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
main.tf Adds trusted_domains variable definition and passes it to the script environment
run.sh Implements domain processing logic and adds trusted domains arguments to code-server command

@Foorack
Copy link
Author

Foorack commented Sep 30, 2025

If I understand these logs correctly, the error has nothing to do with this change?

registry/coder/modules/agentapi/main.test.ts:

registry/thezoker/modules/nodejs/main.test.ts:

7 tests failed:
(fail) jfrog-oauth > can run apply with required variables
(fail) jfrog-token > can run apply with required variables
(fail) github-upload-public-key > creates new key if one does not exist [15002.08ms]
  ^ this test timed out after 15000ms.
(fail) github-upload-public-key > does nothing if one already exists [5001.03ms]
  ^ this test timed out after 5000ms.
(fail) code-server > required variables [135.00ms]
(fail) code-server > use_cached and offline can not be used together [133.00ms]
(fail) code-server > offline and extensions can not be used together [136.00ms]

 376 pass
 7 fail
 2 errors
 1083 expect() calls
Ran 383 tests across 50 files. [453.11s]

@Foorack
Copy link
Author

Foorack commented Sep 30, 2025

Will look into testing more this week, as well as adding tests specifically for this new feature.

@Foorack Foorack force-pushed the link-protection-trusted-domains branch from ccf5b22 to 52099ea Compare September 30, 2025 21:24
@Foorack Foorack changed the title Add trusted_domains variable to code-server module for link protection Draft: Add trusted_domains variable to code-server module for link protection Sep 30, 2025
@Foorack Foorack changed the title Draft: Add trusted_domains variable to code-server module for link protection Add trusted_domains variable to code-server module for link protection Sep 30, 2025
@Foorack Foorack marked this pull request as draft September 30, 2025 21:24
@DevelopmentCats DevelopmentCats marked this pull request as ready for review October 2, 2025 21:53
@DevelopmentCats
Copy link
Contributor

@Foorack Just want to check and see if there is any update?

@DevelopmentCats
Copy link
Contributor

Closing because no response

@Foorack
Copy link
Author

Foorack commented Oct 15, 2025

@DevelopmentCats Apologies with the delay, been busy with work. Can we please re-open the merge request?

I do think it is very hasty to close a Pull Request that is less than a few weeks old. Your update check was also only 20 hours ago. The urgency to get this merged into main reduced once we had a very stable workaround, but I still want to upstream this for the benefit of the greater Coder community.

locals {
  <snip>
  # Trusted domains for code-server link protection
  trusted_domains = [
    "https://open-vsx.org",
    "https://github.com",
    "*.foorack.com",
  ]
}

resource "coder_agent" "main" {
  arch = data.coder_provisioner.me.arch
  os   = "linux"
  dir  = "/home/coder/${local.folder_name}"
  # Add any commands that should be executed at workspace startup (e.g install requirements, start a program, etc) here
  startup_script = <<-EOT
    # Update code-server trusted domains
    echo "🔧 Updating code-server trusted domains..."
    mkdir -p /tmp/code-server/lib
    while [ ! -f "$(find /tmp/code-server/lib/ -type f -name 'product.json' | head -n1)" ]; do :; done
    CODE_SERVER_PROFILE_JSON="$(find /tmp/code-server/lib/ -type f -name 'product.json' | head -n1)"
    cat $CODE_SERVER_PROFILE_JSON | jq '.linkProtectionTrustedDomains = ${jsonencode(local.trusted_domains)}' > /tmp/product-modified.json
    mv /tmp/product-modified.json $CODE_SERVER_PROFILE_JSON

    # Rest of setup...
    <snip>
    EOT

  <snip>
}

@DevelopmentCats
Copy link
Contributor

DevelopmentCats commented Oct 15, 2025

Yeah I can sorry about that!

I generally try to close out PR's that haven't had a response in a few weeks but I'm never against reopening them 😃

I will keep your words in mind though.

Copilot AI and others added 2 commits October 16, 2025 10:10
#1)

* Initial plan

* Add trusted_domains variable to code-server module for link protection

Co-authored-by: Foorack <[email protected]>

* Remove temporary plan files from commit

Co-authored-by: Foorack <[email protected]>

* Refactor TRUSTED_DOMAINS_ARG to match EXTENSION_ARG pattern

Co-authored-by: Foorack <[email protected]>

* Remove trusted domains tests as requested

Co-authored-by: Foorack <[email protected]>

* Fix trusted domains to use multiple flag instances instead of comma-separated values

Co-authored-by: Foorack <[email protected]>

* Update registry/coder/modules/code-server/run.sh

Co-authored-by: Copilot <[email protected]>

* Update registry/coder/modules/code-server/run.sh

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: Foorack <[email protected]>
Co-authored-by: Foorack / Max Faxälv <[email protected]>
Co-authored-by: Copilot <[email protected]>
@Foorack Foorack force-pushed the link-protection-trusted-domains branch from 5e9cbe3 to a6e4529 Compare October 16, 2025 08:10
@Foorack Foorack requested a review from Copilot October 16, 2025 08:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment on lines +19 to +20
# Split comma-separated domains and create multiple --link-protection-trusted-domains arguments
for domain in $(echo "${TRUSTED_DOMAINS}" | tr ',' ' '); do
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

This approach is vulnerable to word splitting and glob expansion issues. If a domain contains spaces or shell metacharacters, it will be incorrectly parsed. Use a more robust parsing method or validate input format.

Suggested change
# Split comma-separated domains and create multiple --link-protection-trusted-domains arguments
for domain in $(echo "${TRUSTED_DOMAINS}" | tr ',' ' '); do
# Split comma-separated domains safely and create multiple --link-protection-trusted-domains arguments
IFS=',' read -ra domains <<< "${TRUSTED_DOMAINS}"
for domain in "${domains[@]}"; do

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants