-
Notifications
You must be signed in to change notification settings - Fork 33
Update sagemaker-extensions-sync to read from environment variables #201
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
Conversation
name: Run unit tests | ||
runs-on: ubuntu-latest | ||
steps: | ||
# Checkout repository code | ||
- name: Checkout code | ||
uses: actions/checkout@v4 | ||
|
||
# Verify CSP line exists in target TypeScript file | ||
- name: Check CSP configuration in webClientServer.ts | ||
run: | | ||
TARGET_FILE="patched-vscode/src/vs/server/node/webClientServer.ts" | ||
REQUIRED_TEXT="'connect-src \'self\' ws: wss: https://main.vscode-cdn.net http://localhost:* https://localhost:* https://login.microsoftonline.com/ https://update.code.visualstudio.com https://*.vscode-unpkg.net/ https://default.exp-tas.com/vscode/ab https://vscode-sync.trafficmanager.net https://vscode-sync-insiders.trafficmanager.net https://*.gallerycdn.vsassets.io https://marketplace.visualstudio.com https://openvsxorg.blob.core.windows.net https://az764295.vo.msecnd.net https://code.visualstudio.com https://*.gallery.vsassets.io https://*.rel.tunnels.api.visualstudio.com wss://*.rel.tunnels.api.visualstudio.com https://*.servicebus.windows.net/ https://vscode.blob.core.windows.net https://vscode.search.windows.net https://vsmarketplacebadges.dev https://vscode.download.prss.microsoft.com https://download.visualstudio.microsoft.com https://*.vscode-unpkg.net https://open-vsx.org;'" | ||
if [ ! -f "$TARGET_FILE" ]; then | ||
echo "❌ FAIL: Target file $TARGET_FILE does not exist." | ||
exit 1 | ||
fi | ||
if grep -F "$REQUIRED_TEXT" "$TARGET_FILE" > /dev/null; then | ||
echo "✅ PASS: Required CSP text exists." | ||
else | ||
echo "❌ FAIL: Required CSP text NOT found in $TARGET_FILE" | ||
exit 1 | ||
fi | ||
# The main job for building the application | ||
build: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 16 days ago
To fix the issue, an explicit permissions
block restricting the GITHUB_TOKEN to only the minimum necessary permissions must be added. Since this workflow primarily interacts with repository contents (reading code for checkout and testing, uploading artifacts, etc.), a "contents: read"
permission is an appropriate minimal base. This block can be placed either at the root level (applying to all jobs, unless a job overrides it) or per job. The clearest and least disruptive fix is to add the following block under the workflow name:
key and before on:
at the top of the file:
permissions:
contents: read
No new methods or imports are required, and no other code is affected. If subsequent jobs require write-level permissions for specific operations, those jobs would override the root permission (not necessary per code shown).
-
Copy modified lines R3-R4
@@ -1,5 +1,7 @@ | ||
# Workflow name | ||
name: Build | ||
permissions: | ||
contents: read | ||
|
||
# This workflow is triggered on pushes and pull requests to the main branch. | ||
on: |
name: Build sagemaker-code-editor | ||
runs-on: ubuntu-latest | ||
# Ensure unit tests pass before building | ||
needs: run-unit-tests | ||
timeout-minutes: 180 | ||
env: | ||
# Environment variable to optimize the build process | ||
DISABLE_V8_COMPILE_CACHE: 1 | ||
|
||
steps: | ||
# Step 1: Check out the repository code, including its submodules. | ||
- name: Checkout repo with submodules | ||
uses: actions/checkout@v4 | ||
with: | ||
submodules: recursive | ||
|
||
# Step 2: Install system-level dependencies required for the build. | ||
- name: Install system dependencies | ||
run: | | ||
sudo apt-get update | ||
sudo apt-get install -y make gcc g++ libx11-dev xorg-dev libxkbfile-dev libsecret-1-dev libkrb5-dev python3 jq perl gettext automake autoconf quilt | ||
# Step 3: Set up the Node.js environment. Version 20 is specified. | ||
- name: Set up Node.js | ||
uses: actions/setup-node@v4 | ||
with: | ||
node-version: 20 | ||
|
||
# Step 4: Cache Yarn dependencies to speed up subsequent builds. | ||
- name: Cache Yarn dependencies | ||
uses: actions/cache@v4 | ||
with: | ||
path: | | ||
vscode/node_modules | ||
key: ${{ runner.os }}-node20-${{ hashFiles('vscode/package.json', 'vscode/yarn.lock') }} | ||
|
||
# Step 5: Apply patches from the 'patches' directory if it exists. | ||
- name: Apply patches (if any) | ||
run: | | ||
if [ -d patches ] && [ "$(ls -A patches)" ]; then | ||
quilt push -a || true | ||
fi | ||
# Step 6: Generate a version string for this specific build. | ||
# It's based on the commit SHA to create a unique identifier. | ||
- name: Set Development Version | ||
id: version | ||
run: | | ||
SHORT_SHA=$(echo "${{ github.sha }}" | cut -c1-7) | ||
VERSION="0.0.0-dev-${SHORT_SHA}" | ||
echo "VERSION=$VERSION" >> $GITHUB_ENV | ||
echo "Generated version for this build: $VERSION" | ||
# Step 7: The main build process for vscode. | ||
- name: Build vscode | ||
run: | | ||
cd vscode | ||
export DISABLE_V8_COMPILE_CACHE=1 | ||
export UV_THREADPOOL_SIZE=4 | ||
npm i -g node-gyp | ||
yarn install --network-concurrency 1 | ||
# Remove and re-add ripgrep | ||
VSCODE_RIPGREP_VERSION=$(jq -r '.dependencies."@vscode/ripgrep"' package.json) | ||
mv package.json package.json.orig | ||
jq 'del(.dependencies."@vscode/ripgrep")' package.json.orig > package.json | ||
# Re-run install to remove ripgrep | ||
yarn install | ||
# Add ripgrep back | ||
yarn add --ignore-scripts "@vscode/ripgrep@${VSCODE_RIPGREP_VERSION}" | ||
ARCH_ALIAS=linux-x64 | ||
# Run the gulp build task | ||
yarn gulp vscode-reh-web-${ARCH_ALIAS}-min | ||
# Step 8: Find the exact path of the original build output directory. | ||
- name: Find build output | ||
id: find_output | ||
run: | | ||
BUILD_PATH=$(find . -name "vscode-reh-web-linux-x64" -type d | head -n 1) | ||
if [ -z "$BUILD_PATH" ]; then | ||
echo "::error::Build output directory 'vscode-reh-web-linux-x64' not found!" | ||
exit 1 | ||
fi | ||
echo "Build output found at: $BUILD_PATH" | ||
echo "build_path=$BUILD_PATH" >> $GITHUB_OUTPUT | ||
# Step 9: Rename the build output directory to sagemaker-code-editor | ||
- name: Rename build output directory | ||
id: rename_output | ||
run: | | ||
ORIG_PATH="${{ steps.find_output.outputs.build_path }}" | ||
PARENT_DIR=$(dirname "$ORIG_PATH") | ||
mv "$ORIG_PATH" "$PARENT_DIR/sagemaker-code-editor" | ||
echo "Renamed build output directory to: $PARENT_DIR/sagemaker-code-editor" | ||
echo "build_path=$PARENT_DIR/sagemaker-code-editor" >> $GITHUB_OUTPUT | ||
# Step 10: Create a compressed tarball of the renamed build output. | ||
- name: Create tarball archive | ||
run: | | ||
TARBALL="sagemaker-code-editor-${{ env.VERSION }}.tar.gz" | ||
BUILD_DIR_PATH="${{ steps.rename_output.outputs.build_path }}" | ||
PARENT_DIR=$(dirname "$BUILD_DIR_PATH") | ||
BUILD_DIR_NAME=$(basename "$BUILD_DIR_PATH") | ||
echo "Creating '$TARBALL' from '$BUILD_DIR_NAME' in '$PARENT_DIR'" | ||
tar czf $TARBALL -C "$PARENT_DIR" "$BUILD_DIR_NAME" | ||
# Step 11: Upload the tarball as a build artifact. | ||
- name: Upload build artifact | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: npm-package | ||
path: sagemaker-code-editor-${{ env.VERSION }}.tar.gz | ||
# Run end-to-end tests after the build is complete | ||
run-e2e-tests: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 16 days ago
The best, simplest fix is to add a permissions:
block at the workflow level (it can also be added per job, but the result is the same unless a particular job requires escalation). This should be at the top level, directly under the workflow name (and above on:
), as recommended by GitHub documentation. The minimal required permission for typical build/test workflows is contents: read
, which allows reading repository contents (needed by actions/checkout).
How to fix:
- Insert the section:
Directly after the workflow
permissions: contents: read
name:
at the top of.github/workflows/build.yml
. - This limits the GITHUB_TOKEN's default access in all jobs to read repository contents, which should be sufficient for the current workflow as shown.
What is needed:
- Only a single permissions block added at the root level of the workflow file.
- No additional imports, methods, or variable definitions are needed.
-
Copy modified lines R3-R4
@@ -1,5 +1,7 @@ | ||
# Workflow name | ||
name: Build | ||
permissions: | ||
contents: read | ||
|
||
# This workflow is triggered on pushes and pull requests to the main branch. | ||
on: |
name: Run e2e tests | ||
runs-on: ubuntu-latest | ||
needs: build # Ensure e2e tests run after build | ||
steps: | ||
# Checkout repository code | ||
- name: Checkout code | ||
uses: actions/checkout@v4 | ||
|
||
# Output placeholder message for e2e tests | ||
- name: Test of e2e test | ||
run: echo "Test of e2e test" |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 16 days ago
To address the issue, we need to explicitly set the permissions:
block in the workflow file. The best fix is to add the permissions:
YAML block at the top level (just below name
and before on
), which will set minimal required permissions (contents: read
) for all jobs in the workflow. This follows the "principle of least privilege" and is the recommended way to prevent the GITHUB_TOKEN from being overprivileged. There are no steps in the shown workflow that require more than contents: read
(no code is being pushed, PRs interacted with, or issues created), so this is safe.
Specifically:
- Add the following block below the
name: Build
:permissions: contents: read
- No changes to steps, imports, or dependencies are needed.
-
Copy modified lines R3-R4
@@ -1,5 +1,7 @@ | ||
# Workflow name | ||
name: Build | ||
permissions: | ||
contents: read | ||
|
||
# This workflow is triggered on pushes and pull requests to the main branch. | ||
on: |
Summary
Test plan
🤖 Generated with Claude Code