Skip to content

feat: add --disable-* options #5

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 5 commits into from
Apr 14, 2025
Merged

feat: add --disable-* options #5

merged 5 commits into from
Apr 14, 2025

Conversation

DanielleMaywood
Copy link
Collaborator

@DanielleMaywood DanielleMaywood commented Apr 14, 2025

This PR adds several new configuration options to the code-server devcontainer feature that allow users to disable specific functionality:

  • disableFileDownloads: Prevents users from downloading files from the editor
  • disableFileUploads: Prevents users from uploading files to the editor
  • disableGettingStartedOverride: Disables the coder/coder override in Help: Getting Started
  • disableProxy: Disables domain and path proxy routes
  • disableTelemetry: Disables telemetry reporting
  • disableUpdateCheck: Disables the update check that runs every 6 hours
  • disableWorkspaceTrust: Disables the Workspace Trust feature for the current session

Add all the '--disable-*' CLI flags as options.
@DanielleMaywood DanielleMaywood requested a review from Copilot April 14, 2025 09:13
Copy link

@Copilot 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.

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

Files not reviewed (11)
  • src/code-server/devcontainer-feature.json: Language not supported
  • src/code-server/install.sh: Language not supported
  • test/code-server/code-server-disable-file-downloads.sh: Language not supported
  • test/code-server/code-server-disable-file-uploads.sh: Language not supported
  • test/code-server/code-server-disable-getting-started-override.sh: Language not supported
  • test/code-server/code-server-disable-multiple-options.sh: Language not supported
  • test/code-server/code-server-disable-proxy.sh: Language not supported
  • test/code-server/code-server-disable-telemetry.sh: Language not supported
  • test/code-server/code-server-disable-update-check.sh: Language not supported
  • test/code-server/code-server-disable-workspace-trust.sh: Language not supported
  • test/code-server/scenarios.json: Language not supported

@DanielleMaywood DanielleMaywood marked this pull request as ready for review April 14, 2025 09:40
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Small suggestion but otherwise LGTM.

@@ -23,12 +23,42 @@ if [[ -n $WORKSPACE ]]; then
CODE_SERVER_WORKSPACE="$WORKSPACE"
fi

DISABLE_FLAGS=""

if [[ "$DISABLEFILEDOWNLOADS" == "true" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Haha, couldn't they have _'d these? 😅

@@ -23,12 +23,42 @@ if [[ -n $WORKSPACE ]]; then
CODE_SERVER_WORKSPACE="$WORKSPACE"
fi

DISABLE_FLAGS=""
Copy link
Member

Choose a reason for hiding this comment

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

Suggestions: using arrays would be slightly cleaner.

args=()
args+=(--disable-file-downloads)
args+=(--disable-file-uploads)

# ...

cmd "${args[*]}"

(Note the use of * vs @ due to how we're creating the entry point, so this does not make it shell-safe when there are spaces in any inputs.)

Copy link
Member

@mafredri mafredri Apr 14, 2025

Choose a reason for hiding this comment

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

Actually, a way to make it shell safe would be:

cat > /usr/local/bin/code-server-entrypoint <<EOF
$(declare -p args)
cmd "\${args[@]}"
EOF

😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we do that second approach do we lose the ability to test the entrypoint for containing certain flags?

Copy link
Member

Choose a reason for hiding this comment

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

Both approaches I outlined should be fine for the test. But in the latter you can't test coder-server.*--flag, but you can test "--flag" individually.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah I understand the second approach more now, I'm happy to do that one 👍

@DanielleMaywood DanielleMaywood merged commit 0825e4c into main Apr 14, 2025
7 checks passed
@DanielleMaywood DanielleMaywood deleted the dm-add-disables branch April 14, 2025 12:52
@DanielleMaywood DanielleMaywood self-assigned this Apr 14, 2025
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.

None yet

2 participants