-
Notifications
You must be signed in to change notification settings - Fork 25
feat(devcontainer): improve performance and reliability of devcontainer launch #119
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
base: main
Are you sure you want to change the base?
Changes from all commits
0873d9f
e38bd50
c60ad7c
94232b2
6345a4a
a8ce6c3
8969f0d
5e3bed2
0c050dc
fc010ee
8443b6f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,28 @@ | ||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # devcontainers copy your local gitconfig but do not parse conditional includes. | ||||||||||||||||||||||
| # This re-configures the devcontainer git identities based on the prior exported | ||||||||||||||||||||||
| # global and local git configurations *after* parsing host includes. See also: | ||||||||||||||||||||||
stewartadam marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||
| # https://github.com/microsoft/vscode-remote-release/issues/2084#issuecomment-2289987894 | ||||||||||||||||||||||
stewartadam marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||
| copy_user_gitconfig() { | ||||||||||||||||||||||
| for conf in .gitconfig.global .gitconfig.local; do | ||||||||||||||||||||||
| if [[ -f "$conf" ]]; then | ||||||||||||||||||||||
| echo "*** Parsing ${conf##.gitconfig.} Git configuration export" | ||||||||||||||||||||||
| while IFS='=' read -r key value; do | ||||||||||||||||||||||
stewartadam marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||
| local key value | ||||||||||||||||||||||
|
Comment on lines
+9
to
+13
|
||||||||||||||||||||||
| for conf in .gitconfig.global .gitconfig.local; do | |
| if [[ -f "$conf" ]]; then | |
| echo "*** Parsing ${conf##.gitconfig.} Git configuration export" | |
| while IFS='=' read -r key value; do | |
| local key value | |
| local key value | |
| for conf in .gitconfig.global .gitconfig.local; do | |
| if [[ -f "$conf" ]]; then | |
| echo "*** Parsing ${conf##.gitconfig.} Git configuration export" | |
| while IFS='=' read -r key value; do |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,10 +5,49 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| set -euo pipefail | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| main() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Volume ownership is not set automatically due to a bug: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # https://github.com/microsoft/vscode-remote-release/issues/9931 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # IMPORTANT: workaround requires Docker base image to have password-less sudo. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fix_volume_ownership() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local volume_path="$1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ ! -d "$volume_path" ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "ERROR: the volume path provided '$volume_path' does not exist." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stewartadam marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Setting volume ownership for $volume_path" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sudo chown "$USER:$USER" "$volume_path" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fix_volume_ownerships() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Applying volume ownership workaround (see microsoft/vscode-remote-release#9931)..." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stewartadam marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fix_volume_ownership "/home/${USER}/.config" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fix_volume_ownership "/workspace/node_modules" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| npm_install() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Installing NPM dependencies..." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| npm install | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "NPM dependencies installed successfully" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| update_ca_certs() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Adds a root CA to the system certificate store. Useful if developer machines | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # have MITM TLS inspection happening, e.g. with ZScaler. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stewartadam marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Updating container system CA certificates..." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if compgen -G ".devcontainer/*.crt" > /dev/null; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sudo cp .devcontainer/*.crt /usr/local/share/ca-certificates/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
stewartadam marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sudo update-ca-certificates | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Container's system CA certificates updated successfully" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+37
to
+44
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Adds a root CA to the system certificate store. Useful if developer machines | |
| # have MITM TLS inspection happening, e.g. with ZScaler. | |
| echo "Updating container system CA certificates..." | |
| if compgen -G ".devcontainer/*.crt" > /dev/null; then | |
| sudo cp .devcontainer/*.crt /usr/local/share/ca-certificates/ | |
| sudo update-ca-certificates | |
| fi | |
| echo "Container's system CA certificates updated successfully" | |
| # Adds a root CA to the system certificate store, but only if it matches a pre-approved whitelist. | |
| # This prevents malicious CAs from being trusted. | |
| echo "Updating container system CA certificates..." | |
| # Define whitelist: filename:sha256sum (add entries as needed) | |
| declare -A CA_CERT_WHITELIST=( | |
| # Example entry (replace with your actual cert and hash): | |
| # ["my-org-root.crt"]="0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" | |
| ) | |
| local found_valid=0 | |
| for certfile in .devcontainer/*.crt; do | |
| if [[ ! -f "${certfile}" ]]; then | |
| continue | |
| fi | |
| certname="$(basename "${certfile}")" | |
| expected_hash="${CA_CERT_WHITELIST[${certname}]:-}" | |
| if [[ -z "${expected_hash}" ]]; then | |
| echo "WARNING: Skipping unapproved CA certificate: ${certname}" | |
| continue | |
| fi | |
| actual_hash="$(sha256sum "${certfile}" | awk '{print $1}')" | |
| if [[ "${actual_hash}" != "${expected_hash}" ]]; then | |
| echo "WARNING: Hash mismatch for ${certname}, skipping." | |
| continue | |
| fi | |
| echo "Installing approved CA certificate: ${certname}" | |
| sudo cp "${certfile}" /usr/local/share/ca-certificates/ | |
| found_valid=1 | |
| done | |
| if (( found_valid )); then | |
| sudo update-ca-certificates | |
| echo "Container's system CA certificates updated successfully" | |
| else | |
| echo "No valid CA certificates installed." | |
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WilliamBerryiii thoughts? I'm inclined to consider this a non-risk, if an attacker has access to the repo they can do far worse damage by updating any of the initialization scripts for the dev container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would love perspective from @agreaves-ms on this ... I'm on the fence here (mainly because I'm on windows and mucking with the cert store can be a PITA if things go sideways).
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| **/.git/ | ||
| **/node_modules/ |
Uh oh!
There was an error while loading. Please reload this page.