Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 43 additions & 7 deletions .github/workflows/bump-package-version.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,60 @@ jobs:
run: node dist/generateOobeeClientScanner.js

- name: Create branch and commit changes
env:
NEW_VERSION: ${{ steps.bump.outputs.version }}
CURRENT_VERSION: ${{ steps.current.outputs.version }}
run: |
BRANCH="bump/version-${{ steps.bump.outputs.version }}"
# Validate versions to a strict allowlist before use in shell.
case "$NEW_VERSION" in
v[0-9]*.[0-9]*.[0-9]*) ;;
[0-9]*.[0-9]*.[0-9]*) ;;
*) echo "Unexpected NEW_VERSION: $NEW_VERSION" >&2; exit 1 ;;
esac
case "$CURRENT_VERSION" in
v[0-9]*.[0-9]*.[0-9]*) ;;
[0-9]*.[0-9]*.[0-9]*) ;;
*) echo "Unexpected CURRENT_VERSION: $CURRENT_VERSION" >&2; exit 1 ;;
esac
BRANCH="bump/version-$NEW_VERSION"
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
git checkout -b "$BRANCH"
git add package.json package-lock.json oobee-client-scanner.js oobee-client-scanner.js.sha384
git commit -m "chore: bump version ${{ steps.current.outputs.version }} → ${{ steps.bump.outputs.version }}"
git commit -m "chore: bump version $CURRENT_VERSION → $NEW_VERSION"
git push origin "$BRANCH"
echo "BRANCH=$BRANCH" >> $GITHUB_ENV
echo "BRANCH=$BRANCH" >> "$GITHUB_ENV"

- name: Authenticate GitHub CLI
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
echo "${{ secrets.GITHUB_TOKEN }}" | gh auth login --with-token
echo "$GH_TOKEN" | gh auth login --with-token

- name: Create pull request
env:
NEW_VERSION: ${{ steps.bump.outputs.version }}
CURRENT_VERSION: ${{ steps.current.outputs.version }}
BASE_REF: ${{ github.ref_name }}
run: |
# Re-validate here since env vars are scoped per step.
case "$NEW_VERSION" in
v[0-9]*.[0-9]*.[0-9]*) ;;
[0-9]*.[0-9]*.[0-9]*) ;;
*) echo "Unexpected NEW_VERSION: $NEW_VERSION" >&2; exit 1 ;;
esac
case "$CURRENT_VERSION" in
v[0-9]*.[0-9]*.[0-9]*) ;;
[0-9]*.[0-9]*.[0-9]*) ;;
*) echo "Unexpected CURRENT_VERSION: $CURRENT_VERSION" >&2; exit 1 ;;
esac
# Restrict base ref to safe characters (letters, digits, dot, underscore, dash, slash).
case "$BASE_REF" in
*[!A-Za-z0-9._/-]*) echo "Unsafe BASE_REF: $BASE_REF" >&2; exit 1 ;;
"") echo "Empty BASE_REF" >&2; exit 1 ;;
esac
gh pr create \
--title "chore: bump version to ${{ steps.bump.outputs.version }}" \
--body "Automated version bump from ${{ steps.current.outputs.version }} to ${{ steps.bump.outputs.version }}." \
--title "chore: bump version to $NEW_VERSION" \
--body "Automated version bump from $CURRENT_VERSION to $NEW_VERSION." \
--head "$BRANCH" \
--base "${{ github.ref_name }}"
--base "$BASE_REF"
37 changes: 34 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,44 @@
# Chrome .deb packages are available for amd64 (x86_64) and arm64 (aarch64).
# Other architectures will skip this step and Safe Browsing will not be available.
#
# INTEGRITY (asgard-0009): We no longer download the "current" .deb directly
# from dl.google.com, which had no independent integrity check. Instead we
# register Google's signed apt repository with the Linux signing key, and
# install google-chrome-stable via apt so every package is verified
# against Google's release signature by apt itself. Applies uniformly to
# amd64 and arm64 (Google ships both from the same signed repo).
# Optionally set --build-arg GOOGLE_CHROME_SIGNING_KEY_SHA256=<sha256> to
# additionally pin the fetched signing key against a known SHA-256; the
# default (empty) trusts TLS to dl.google.com for the key itself and lets
# apt's GPG verification catch package tampering.
#
# TO ENABLE: Set env var GOOGLE_SAFE_BROWSING=1 when running the container.
# =============================================================================
ARG GOOGLE_CHROME_SIGNING_KEY_SHA256=

Check warning on line 38 in Dockerfile

View workflow job for this annotation

GitHub Actions / build (linux/amd64, ubuntu-latest)

Sensitive data should not be used in the ARG or ENV commands

SecretsUsedInArgOrEnv: Do not use ARG or ENV instructions for sensitive data (ARG "GOOGLE_CHROME_SIGNING_KEY_SHA256") More info: https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/

Check warning on line 38 in Dockerfile

View workflow job for this annotation

GitHub Actions / build (linux/arm64, ubuntu-24.04-arm)

Sensitive data should not be used in the ARG or ENV commands

SecretsUsedInArgOrEnv: Do not use ARG or ENV instructions for sensitive data (ARG "GOOGLE_CHROME_SIGNING_KEY_SHA256") More info: https://docs.docker.com/go/dockerfile/rule/secrets-used-in-arg-or-env/
RUN ARCH="$(dpkg --print-architecture)"; \
if [ "$ARCH" = "amd64" ] || [ "$ARCH" = "arm64" ]; then \
wget -q -O /tmp/chrome.deb "https://dl.google.com/linux/direct/google-chrome-stable_current_${ARCH}.deb" && \
apt-get update && apt-get install -y --no-install-recommends /tmp/chrome.deb && \
rm -f /tmp/chrome.deb && rm -rf /var/lib/apt/lists/*; \
set -e; \
TMPKEY="$(mktemp)"; \
wget -q -O "$TMPKEY" https://dl.google.com/linux/linux_signing_key.pub; \
if [ -n "$GOOGLE_CHROME_SIGNING_KEY_SHA256" ]; then \
ACTUAL_KEY_SHA256="$(sha256sum "$TMPKEY" | awk '{print $1}')"; \
if [ "$ACTUAL_KEY_SHA256" != "$GOOGLE_CHROME_SIGNING_KEY_SHA256" ]; then \
echo "ERROR: Google Chrome signing key SHA-256 mismatch"; \
echo " expected: $GOOGLE_CHROME_SIGNING_KEY_SHA256"; \
echo " actual: $ACTUAL_KEY_SHA256"; \
rm -f "$TMPKEY"; \
exit 1; \
fi; \
fi; \
install -d -m 0755 /usr/share/keyrings; \
install -m 0644 "$TMPKEY" /usr/share/keyrings/google-chrome.asc; \
rm -f "$TMPKEY"; \
echo "deb [arch=${ARCH} signed-by=/usr/share/keyrings/google-chrome.asc] https://dl.google.com/linux/chrome/deb/ stable main" \
> /etc/apt/sources.list.d/google-chrome.list; \
apt-get update && \
apt-get install -y --no-install-recommends google-chrome-stable && \
apt-mark hold google-chrome-stable && \
rm -rf /var/lib/apt/lists/*; \
else \
echo "NOTICE: Skipping Chrome install (Safe Browsing unavailable on $ARCH)"; \
fi
Expand Down
27 changes: 24 additions & 3 deletions scripts/install_oobee_dependencies.command
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,30 @@ if ! [ -f verapdf/verapdf ]; then
curl -fSL -o ./verapdf-installer.zip https://github.com/GovTechSG/oobee/releases/download/cache/verapdf-installer.zip
verify_sha256 ./verapdf-installer.zip "$VERAPDF_SHA256" "veraPDF installer"
unzip -j ./verapdf-installer.zip -d ./verapdf-installer
./verapdf-installer/verapdf-install "${__dir}/verapdf-auto-install-macos.xml"
cp -r /tmp/verapdf .
rm -rf ./verapdf-installer.zip ./verapdf-installer /tmp/verapdf

# Stage the veraPDF install into a private per-run directory (mode 0700)
# rather than the shared, predictable /tmp/verapdf path — a co-located
# local user could otherwise pre-plant or symlink /tmp/verapdf and get
# their code copied into oobee's PATH (asgard-0003).
VERAPDF_STAGE_ROOT="$(mktemp -d "${TMPDIR:-/tmp}/oobee-verapdf.XXXXXXXXXX")"
chmod 700 "$VERAPDF_STAGE_ROOT"
cleanup_verapdf_stage() { rm -rf "$VERAPDF_STAGE_ROOT"; }
trap cleanup_verapdf_stage EXIT
VERAPDF_STAGE_DIR="$VERAPDF_STAGE_ROOT/verapdf"
VERAPDF_AUTO_XML="$VERAPDF_STAGE_ROOT/verapdf-auto-install-macos.xml"
# sed -e with a placeholder token avoids embedding user-derived paths as
# regex or replacement metacharacters.
awk -v repl="$VERAPDF_STAGE_DIR" '{ gsub(/@INSTALLPATH@/, repl); print }' \
"${__dir}/verapdf-auto-install-macos.xml" > "$VERAPDF_AUTO_XML"
./verapdf-installer/verapdf-install "$VERAPDF_AUTO_XML"
if [ ! -d "$VERAPDF_STAGE_DIR" ]; then
echo "ERROR: veraPDF install did not produce expected output at $VERAPDF_STAGE_DIR" >&2
exit 1
fi
cp -r "$VERAPDF_STAGE_DIR" .
cleanup_verapdf_stage
trap - EXIT
rm -rf ./verapdf-installer.zip ./verapdf-installer

fi

Expand Down
23 changes: 21 additions & 2 deletions scripts/install_oobee_dependencies.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,27 @@ if (-Not (Test-Path verapdf\verapdf.bat)) {
$env:Path = "$env:JAVA_HOME\bin;$env:Path"

Write-Output "INFO: Installing VeraPDF"
.\verapdf-installer\verapdf-install "$PWD\verapdf-auto-install-windows.xml"
Move-Item -Path C:\Windows\Temp\verapdf -Destination verapdf
# Stage the veraPDF install into a per-user, non-predictable directory
# under $env:TEMP rather than C:\Windows\Temp (world-writable on standard
# Windows configs — asgard-0002). Rewrite the automated-install XML on
# the fly so izpack writes into the private path.
$veraStageRoot = Join-Path $env:TEMP ([System.IO.Path]::GetRandomFileName())
New-Item -ItemType Directory -Path $veraStageRoot | Out-Null
$veraStageDir = Join-Path $veraStageRoot "verapdf"
$veraAutoXml = Join-Path $veraStageRoot "verapdf-auto-install-windows.xml"
try {
(Get-Content -Raw -LiteralPath "$PWD\verapdf-auto-install-windows.xml") `
-replace [regex]::Escape('@INSTALLPATH@'), $veraStageDir `
| Set-Content -LiteralPath $veraAutoXml -Encoding UTF8
.\verapdf-installer\verapdf-install $veraAutoXml
if (-not (Test-Path $veraStageDir)) {
Write-Error "veraPDF install did not produce expected output at $veraStageDir"
exit 1
}
Move-Item -Path $veraStageDir -Destination verapdf
} finally {
Remove-Item -Force -Recurse -ErrorAction SilentlyContinue $veraStageRoot
}
Remove-Item -Force -Path .\verapdf-installer.zip
Remove-Item -Force -Path .\verapdf-installer -recurse
}
Expand Down
15 changes: 14 additions & 1 deletion scripts/oobee_shell.sh
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,20 @@ export PLAYWRIGHT_BROWSERS_PATH="$PWD/ms-playwright"
export PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD="true"

echo "INFO: Removing com.apple.quarantine attributes for required binaries to run"
xattr -rd com.apple.quarantine . &>/dev/null
# Only strip Gatekeeper quarantine from the specific tool trees that oobee
# itself just installed and verified — not the whole working directory, which
# would also unquarantine any attacker-planted files that happened to land
# alongside them (part of the asgard-0003 hardening).
for _oobee_quarantine_dir in \
"$PWD/nodejs-mac-arm64" \
"$PWD/nodejs-mac-x64" \
"$PWD/jre" \
"$PWD/verapdf"; do
if [ -e "$_oobee_quarantine_dir" ]; then
xattr -rd com.apple.quarantine "$_oobee_quarantine_dir" &>/dev/null || true
fi
done
unset _oobee_quarantine_dir

cd "$ORIGINAL_DIR"
$@
2 changes: 1 addition & 1 deletion scripts/verapdf-auto-install-macos.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<AutomatedInstallation langpack="eng">
<com.izforge.izpack.panels.htmlhello.HTMLHelloPanel id="welcome"/>
<com.izforge.izpack.panels.target.TargetPanel id="install_dir">
<installpath>/tmp/verapdf</installpath>
<installpath>@INSTALLPATH@</installpath>
</com.izforge.izpack.panels.target.TargetPanel>
<com.izforge.izpack.panels.packs.PacksPanel id="sdk_pack_select">
<pack index="0" name="veraPDF GUI" selected="true"/>
Expand Down
2 changes: 1 addition & 1 deletion scripts/verapdf-auto-install-windows.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<AutomatedInstallation langpack="eng">
<com.izforge.izpack.panels.htmlhello.HTMLHelloPanel id="welcome"/>
<com.izforge.izpack.panels.target.TargetPanel id="install_dir">
<installpath>C:\Windows\Temp\verapdf</installpath>
<installpath>@INSTALLPATH@</installpath>
</com.izforge.izpack.panels.target.TargetPanel>
<com.izforge.izpack.panels.packs.PacksPanel id="sdk_pack_select">
<pack index="0" name="veraPDF GUI" selected="true"/>
Expand Down
39 changes: 38 additions & 1 deletion src/constants/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,24 @@ export const checkUrlConnectivityWithBrowser = async (
let contentType = '';
const protocol = new URL(url).protocol;

// SSRF defence (asgard-0004): opt-in block of seed URLs that resolve to
// loopback/link-local/RFC1918/CGNAT/metadata addresses. Off by default
// so CLI users can continue to scan localhost / staging targets on
// purpose; hosted deployments that accept unauthenticated caller-
// supplied URLs should set OOBEE_BLOCK_INTERNAL_TARGETS=1. Discovered
// links inside the crawl are always filtered — see the preNavigation
// hook in crawlDomain.ts, which closes the primary vector (attacker
// subdomains resolving to internal IPs on scanned third-party sites).
if (
(protocol === 'http:' || protocol === 'https:') &&
/^(1|true|yes)$/i.test(process.env.OOBEE_BLOCK_INTERNAL_TARGETS ?? '')
) {
if (await isInternalOrLoopbackUrl(url)) {
res.status = constants.urlCheckStatuses.invalidUrl.code;
return res;
}
}

if (protocol !== 'http:' && protocol !== 'https:') {
try {
const filePath = fileURLToPath(url);
Expand Down Expand Up @@ -1083,7 +1101,26 @@ const ipv4InRange = (ip: string, cidr: string): boolean => {
};
const isInternalIpv4 = (ip: string): boolean =>
INTERNAL_ADDR_RANGES.some(r => ipv4InRange(ip, r));
async function isInternalOrLoopbackUrl(candidate: string): Promise<boolean> {
// Returns true when the given remote IP (as reported by
// Playwright's Response.serverAddr().ipAddress) falls in a loopback,
// link-local, RFC1918, CGNAT, or metadata-service range. Used as a
// post-navigation DNS-rebinding check alongside the pre-navigation
// isInternalOrLoopbackUrl() host filter.
export const isInternalRemoteIp = (remoteIp: string | undefined | null): boolean => {
if (!remoteIp) return false;
const bare = remoteIp.replace(/^\[|\]$/g, '').toLowerCase();
return (
(isIpv4Literal(bare) && isInternalIpv4(bare)) ||
bare === '::1' ||
bare.startsWith('fe8') || bare.startsWith('fe9') ||
bare.startsWith('fea') || bare.startsWith('feb') ||
bare.startsWith('fc') || bare.startsWith('fd') ||
bare.startsWith('::ffff:127.') || bare.startsWith('::ffff:10.') ||
bare.startsWith('::ffff:169.254.') || bare.startsWith('::ffff:192.168.')
);
};

export async function isInternalOrLoopbackUrl(candidate: string): Promise<boolean> {
let host: string;
try {
host = new URL(candidate).hostname;
Expand Down
42 changes: 41 additions & 1 deletion src/crawlers/crawlDomain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import {
isDisallowedInRobotsTxt,
getUrlsFromRobotsTxt,
waitForPageLoaded,
isInternalOrLoopbackUrl,
isInternalRemoteIp,
} from '../constants/common.js';
import { areLinksEqual, isFollowStrategy, isSameHostname, normUrl, register } from '../utils.js';
import {
Expand Down Expand Up @@ -474,6 +476,18 @@ const crawlDomain = async ({
const ext = parsed.pathname.toLowerCase().split('.').pop();
if (ext && blackListedFileExtensions.includes(ext)) {
request.skipNavigation = true;
return;
}
// SSRF defence: block navigation to loopback / link-local /
// RFC1918 / cloud-metadata destinations (asgard-0004). An
// attacker-controlled same-registrable-domain subdomain can
// otherwise resolve to 169.254.169.254 / 127.0.0.1 / an
// internal RFC1918 host and be fetched by the crawler.
if (await isInternalOrLoopbackUrl(request.url)) {
consoleLogger.warn(
`Refusing to navigate to internal/loopback address for ${request.url}`,
);
request.skipNavigation = true;
}
} catch {
request.skipNavigation = true;
Expand All @@ -482,7 +496,33 @@ const crawlDomain = async ({
],
postNavigationHooks: [
async crawlingContext => {
const { page, request } = crawlingContext;
const { page, request, response } = crawlingContext as PlaywrightCrawlingContext;

// DNS-rebinding defence (asgard-0004): a hostile authoritative
// DNS can return a public IP to isInternalOrLoopbackUrl() and a
// private IP to the browser milliseconds later. Verify the
// remote address the browser actually connected to falls outside
// internal ranges before axe/capturePageData runs over the body.
if (response) {
try {
const serverAddr = await response.serverAddr();
if (isInternalRemoteIp(serverAddr?.ipAddress)) {
consoleLogger.warn(
`Refusing response from internal address ${serverAddr?.ipAddress} for ${request.url}`,
);
request.skipNavigation = true;
try {
await page.goto('about:blank', { timeout: 5000 });
} catch {
// best-effort — the request handler will already skip.
}
return;
}
} catch {
// serverAddr() is best-effort — Chromium may not report it
// for service-worker/cached responses.
}
}

try {
await page.evaluate(() => {
Expand Down
12 changes: 6 additions & 6 deletions src/crawlers/crawlSitemap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,18 +150,18 @@ const crawlSitemap = async ({
const { maxConcurrency } = constants;
// Bind Basic-auth credentials to the entry URL's origin so Playwright
// won't auto-attach them after a cross-origin redirect (credential leak).
const { nonAuthHeaders, httpCredentials } = splitAuthHeaders(
const { authHeader, nonAuthHeaders, httpCredentials } = splitAuthHeaders(
extraHTTPHeaders,
userUrl || sitemapUrl,
);

// Never send caller-supplied credentials to a server whose certificate
// couldn't be validated (asgard-0006). Matches the runCustom /
// couldn't be validated (asgard-0006 / asgard-0005). Matches the runCustom /
// launchPersistentSafeContext safe pattern: hold TLS validation ON whenever
// credentials are attached, and require an explicit opt-in env var for
// credential-less scans that legitimately need to reach hosts with broken
// certs.
const hasCredentials = !!httpCredentials;
// ANY auth material is attached — not only Basic (which becomes
// httpCredentials), but also Bearer / custom Authorization schemes that
// splitAuthHeaders leaves in authHeader/extraHTTPHeaders.
const hasCredentials = !!authHeader || !!httpCredentials;
const allowInsecureTls =
!hasCredentials &&
['1', 'true', 'yes'].includes(
Expand Down
7 changes: 7 additions & 0 deletions src/mergeAxeResults/sentryTelemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ const sendWcagBreakdownToSentry = async (
allIssues?: AllIssues,
pagesScannedCount: number = 0,
) => {
// Honor the OOBEE_DISABLE_TELEMETRY opt-out (asgard-0007). The parallel
// Google-Sheets submission in constants/common.ts's submitForm() checks the
// same flag; without this guard, PII (email, name, entry URL) is still sent
// to Sentry even when the user has explicitly opted out.
if (/^(1|true|yes)$/i.test(process.env.OOBEE_DISABLE_TELEMETRY ?? '')) {
return;
}
try {
// Initialize Sentry
Sentry.init(sentryConfig);
Expand Down
10 changes: 9 additions & 1 deletion src/static/ejs/partials/scripts/ruleModal/utilities.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,15 @@
? 'Unable to connect to AI service. Please ensure the wcag-eval API is running on port 5000.'
: `Failed to generate fix suggestion: ${error.message}`;

errorContainer.innerHTML = `<div class="generateAiError">${errorMessage}</div>`;
// Render error text as a text node (asgard-0006): error.message can
// contain attacker-influenced content when the external AI service
// reflects submitted HTML back in its error field. The success path
// (formatFixSuggestionResponse) already escapes every field before
// innerHTML; do the same here.
const errorDiv = document.createElement('div');
errorDiv.className = 'generateAiError';
errorDiv.textContent = errorMessage;
errorContainer.replaceChildren(errorDiv);
}
}
};
Expand Down
Loading