Skip to content

Commit 832ba4e

Browse files
delilahwdelilahw
committed
feat: add executable wrappers
Wrap the underlying tools with an executable script, rather than relying on shell functions or aliases. This means that these scenarios will now work: - Invoking `npx` from a JS script, e.g. during a pre-commit hook - Invoking ```py popen(['npx'], shell=False) ```
1 parent 995c7d9 commit 832ba4e

5 files changed

Lines changed: 159 additions & 24 deletions

File tree

‎src/artifacts-helper/codespaces_artifacts_helper_keyring/.vscode/settings.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"[python]": {
3+
"editor.rulers": [
4+
88,
5+
]
6+
},
7+
"[toml]": {
8+
"editor.formatOnSave": false
9+
}
10+
}

‎src/artifacts-helper/devcontainer-feature.json‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,24 @@
5858
"type": "boolean",
5959
"default": false,
6060
"description": "Install Python keyring helper for pip"
61+
},
62+
"wrapperType": {
63+
"type": "string",
64+
"default": "SHELL_FUNCTION",
65+
"description": "Type of wrapper script to use.",
66+
"enum": [
67+
"SHELL_FUNCTION",
68+
"EXECUTABLE"
69+
]
6170
}
6271
},
72+
"containerEnv": {
73+
"PATH": "/usr/local/share/codespace-features:${PATH}"
74+
},
6375
"installsAfter": [
6476
"ghcr.io/devcontainers/features/common-utils",
65-
"ghcr.io/devcontainers/features/python"
77+
"ghcr.io/devcontainers/features/python",
78+
"ghcr.io/devcontainers/features/node"
6679
],
6780
"customizations": {
6881
"vscode": {

‎src/artifacts-helper/install.sh‎

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ set -e
44

55
PREFIXES="${NUGETURIPREFIXES:-"https://pkgs.dev.azure.com/"}"
66
USENET6="${DOTNET6:-"false"}"
7+
WRAPPERTYPE="${WRAPPERTYPE:-"SHELL_FUNCTION"}"
78
ALIAS_DOTNET="${DOTNETALIAS:-"true"}"
89
ALIAS_NUGET="${NUGETALIAS:-"true"}"
910
ALIAS_NPM="${NPMALIAS:-"true"}"
@@ -14,30 +15,35 @@ ALIAS_PNPM="${PNPMALIAS:-"true"}"
1415
INSTALL_PIP_HELPER="${PYTHON:-"false"}"
1516
COMMA_SEP_TARGET_FILES="${TARGETFILES:-"DEFAULT"}"
1617

17-
ALIASES_ARR=()
18+
# Destination to install wrappers for the `EXECUTABLE` wrapper type.
19+
# This path must take precedence over /usr/local/bin and the original tools,
20+
# which is ensured via `containerEnv.PATH` in devcontainer-feature.json.
21+
EXECUTABLES_TARGET_DIR='/usr/local/share/codespace-features'
22+
23+
WRAPPED_BINS_ARR=()
1824

1925
if [ "${ALIAS_DOTNET}" = "true" ]; then
20-
ALIASES_ARR+=('dotnet')
26+
WRAPPED_BINS_ARR+=('dotnet')
2127
fi
2228
if [ "${ALIAS_NUGET}" = "true" ]; then
23-
ALIASES_ARR+=('nuget')
29+
WRAPPED_BINS_ARR+=('nuget')
2430
fi
2531
if [ "${ALIAS_NPM}" = "true" ]; then
26-
ALIASES_ARR+=('npm')
32+
WRAPPED_BINS_ARR+=('npm')
2733
fi
2834
if [ "${ALIAS_YARN}" = "true" ]; then
29-
ALIASES_ARR+=('yarn')
35+
WRAPPED_BINS_ARR+=('yarn')
3036
fi
3137
if [ "${ALIAS_NPX}" = "true" ]; then
32-
ALIASES_ARR+=('npx')
38+
WRAPPED_BINS_ARR+=('npx')
3339
fi
3440
if [ "${ALIAS_RUSH}" = "true" ]; then
35-
ALIASES_ARR+=('rush')
36-
ALIASES_ARR+=('rush-pnpm')
41+
WRAPPED_BINS_ARR+=('rush')
42+
WRAPPED_BINS_ARR+=('rush-pnpm')
3743
fi
3844
if [ "${ALIAS_PNPM}" = "true" ]; then
39-
ALIASES_ARR+=('pnpm')
40-
ALIASES_ARR+=('pnpx')
45+
WRAPPED_BINS_ARR+=('pnpm')
46+
WRAPPED_BINS_ARR+=('pnpx')
4147
fi
4248

4349
# Source /etc/os-release to get OS info
@@ -116,27 +122,45 @@ if command -v sudo >/dev/null 2>&1; then
116122
fi
117123
fi
118124

119-
if [ "${COMMA_SEP_TARGET_FILES}" = "DEFAULT" ]; then
120-
if [ "${INSTALL_WITH_SUDO}" = "true" ]; then
121-
COMMA_SEP_TARGET_FILES="~/.bashrc,~/.zshrc"
122-
else
123-
COMMA_SEP_TARGET_FILES="/etc/bash.bashrc,/etc/zsh/zshrc"
125+
if [ "$WRAPPERTYPE" = "SHELL_FUNCTION" ]; then
126+
echo "Installing shell functions for wrapped commands."
127+
128+
if [ "${COMMA_SEP_TARGET_FILES}" = "DEFAULT" ]; then
129+
if [ "${INSTALL_WITH_SUDO}" = "true" ]; then
130+
COMMA_SEP_TARGET_FILES="~/.bashrc,~/.zshrc"
131+
else
132+
COMMA_SEP_TARGET_FILES="/etc/bash.bashrc,/etc/zsh/zshrc"
133+
fi
124134
fi
125-
fi
126135

127-
IFS=',' read -r -a TARGET_FILES_ARR <<< "$COMMA_SEP_TARGET_FILES"
136+
IFS=',' read -r -a TARGET_FILES_ARR <<< "$COMMA_SEP_TARGET_FILES"
128137

129-
for ALIAS in "${ALIASES_ARR[@]}"; do
130-
for TARGET_FILE in "${TARGET_FILES_ARR[@]}"; do
131-
CMD="$ALIAS() { /usr/local/bin/run-$ALIAS.sh \"\$@\"; }"
138+
for ALIAS in "${WRAPPED_BINS_ARR[@]}"; do
139+
for TARGET_FILE in "${TARGET_FILES_ARR[@]}"; do
140+
CMD="$ALIAS() { /usr/local/bin/run-$ALIAS.sh \"\$@\"; }"
132141

142+
if [ "${INSTALL_WITH_SUDO}" = "true" ]; then
143+
sudo -u ${_REMOTE_USER} bash -c "echo '$CMD' >> $TARGET_FILE"
144+
else
145+
echo $CMD >> $TARGET_FILE || true
146+
fi
147+
done
148+
done
149+
elif [ "$WRAPPERTYPE" = "EXECUTABLE" ]; then
150+
echo "Installing executable scripts for wrapped commands."
151+
152+
for ALIAS in "${WRAPPED_BINS_ARR[@]}"; do
153+
CMD_LINE=(ln -s "/usr/local/bin/run-$ALIAS.sh" "${EXECUTABLES_TARGET_DIR}/$ALIAS")
133154
if [ "${INSTALL_WITH_SUDO}" = "true" ]; then
134-
sudo -u ${_REMOTE_USER} bash -c "echo '$CMD' >> $TARGET_FILE"
155+
sudo -u "${_REMOTE_USER}" "${CMD_LINE[@]}"
135156
else
136-
echo $CMD >> $TARGET_FILE || true
157+
"${CMD_LINE[@]}"
137158
fi
138159
done
139-
done
160+
else
161+
echo "Invalid WRAPPERTYPE specified: $WRAPPERTYPE. Must be one of SHELL_FUNCTION or EXECUTABLE."
162+
exit 1
163+
fi
140164

141165
if [ "${INSTALL_WITH_SUDO}" = "true" ]; then
142166
sudo -u ${_REMOTE_USER} bash -c "/tmp/install-provider.sh ${USENET6}"

‎test/artifacts-helper/scenario_executable_wrapper.sh‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
#!/usr/bin/env bash
2+
3+
set -e
4+
5+
WRAPPER_INSTALL_PATH='/usr/local/share/codespace-features'
6+
BINS_TO_CHECK=('yarn' 'npm' 'npx')
7+
8+
check_path_priority() {
9+
log "Checking PATH priority"
10+
11+
for BIN_NAME in "${BINS_TO_CHECK[@]}"; do
12+
if ! command -v "$BIN_NAME" &>/dev/null; then
13+
log "Error: $BIN_NAME not found in PATH"
14+
exit 1
15+
fi
16+
17+
# Check if the target binary is wrapped
18+
ACTUAL_BIN_PATH=$(command -v "$BIN_NAME")
19+
EXPECTED_BIN_PATH="$WRAPPER_INSTALL_PATH/$BIN_NAME"
20+
if ! grep -q "$EXPECTED_BIN_PATH" <<<"$ACTUAL_BIN_PATH"; then
21+
log "Error: $BIN_NAME is not wrapped. We expected $EXPECTED_BIN_PATH but the actual binary path is $ACTUAL_BIN_PATH."
22+
log "Please contact Clipchamp EngProd and let them know that the feed-auth-wrapper feature is not working as expected."
23+
exit 1
24+
fi
25+
26+
log "Success: $BIN_NAME is wrapped."
27+
done
28+
}
29+
30+
check_bin_exec() {
31+
log "Checking if the wrapped binaries get executed"
32+
33+
WRAPPED_BINS_DIR=$(mktemp -d)
34+
BASH_BIN_DIR=$(dirname "$(command -v bash)")
35+
TEST_PATH="$WRAPPER_INSTALL_PATH:$WRAPPER_INSTALL_PATH:$WRAPPED_BINS_DIR:$BASH_BIN_DIR"
36+
37+
for BIN_NAME in "${BINS_TO_CHECK[@]}"; do
38+
log "Checking $BIN_NAME"
39+
log "Creating a temporary binary to be shadowed by the wrapper"
40+
WRAPPED_BIN="$WRAPPED_BINS_DIR/$BIN_NAME"
41+
expected_stdout="Hello from $BIN_NAME"
42+
cat <<EOF >"$WRAPPED_BIN"
43+
#!/usr/bin/env bash
44+
if [ -z "\$ARTIFACTS_ACCESSTOKEN" ]; then
45+
echo "Error: ARTIFACTS_ACCESSTOKEN was not set! It should be set by the artifacts-helper wrapper."
46+
exit 1
47+
fi
48+
echo "$expected_stdout"
49+
EOF
50+
chmod +x "$WRAPPED_BIN"
51+
52+
log "Executing $BIN_NAME to check if it wraps the temporary binary"
53+
actual_stdout=$(PATH="$TEST_PATH" "$BIN_NAME")
54+
actual_stderr=$(PATH="$TEST_PATH" "$BIN_NAME" 2>&1 > /dev/null)
55+
56+
log "Checking the output of the wrapper"
57+
log "stdout: $actual_stdout"
58+
log "stderr: $actual_stderr"
59+
if [ "$actual_stdout" = "$expected_stdout" ]; then
60+
log "Success: wrapper for $BIN_NAME executed correctly."
61+
else
62+
log "Error: wrapper for $BIN_NAME did not execute correctly. Expected '$expected_stdout' but got '$actual_stdout'."
63+
exit 1
64+
fi
65+
done
66+
67+
rm -r "$WRAPPED_BINS_DIR"
68+
log "Success: Wrapped binaries are executed correctly."
69+
}
70+
71+
main() {
72+
echo "Starting scenario_executable_wrapper tests"
73+
74+
check_path_priority
75+
check_bin_exec
76+
77+
echo "scenario_executable_wrapper tests completed successfully"
78+
}
79+
80+
main

‎test/artifacts-helper/scenarios.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,13 @@
4242
"python": false
4343
}
4444
}
45+
},
46+
"scenario_executable_wrapper": {
47+
"image": "mcr.microsoft.com/devcontainers/base:debian",
48+
"features": {
49+
"artifacts-helper": {
50+
"wrapperType": "EXECUTABLE"
51+
}
52+
}
4553
}
4654
}

0 commit comments

Comments
 (0)