From fcdac79dabfe36860a2f62a2afcb54523f21200e Mon Sep 17 00:00:00 2001 From: Jeffrey Aven Date: Thu, 1 Oct 2026 08:54:19 +1000 Subject: [PATCH] harden CI pipeline against contributor-controlled names and symlinks Contributor-controlled data (branch names, PR titles, provider directory and service file names) reached /bin/sh via exec/os.system in the setup and signing scripts, and symlinks under providers/src were dereferenced by the update, sign and package steps of the post-merge build. - setup-job.js / get-version.js: write REG_* via core.exportVariable, parse the PR number with an anchored regex for GitHub merge and squash subjects and require it to be numeric - get-updated-providers.py: write PROVIDERS / NUM_PROVIDERS directly to $GITHUB_ENV in heredoc form; accept only providers/src///{provider.yaml,services/} with every component matching [A-Za-z0-9_][A-Za-z0-9._-]* - new scripts/common/provider_tree.py: shared name allowlist, symlink / regular-file / containment checks, GITHUB_ENV writer, S3 key parser - update-versions.py, sign-provider-docs.py: validate the provider tree before reading it; shutil.copyfile instead of os.system("cp ...") - simulate-REGISTRY-PULL.py: tarfile extractall with filter='data' - pull-additional-docs-from-artifact-repo.py: validate artifact keys before joining them onto local paths - sign-file.sh, main.yml: quote arguments - CONTRIBUTING.md: document the enforced layout and naming rules All 38 existing providers (1909 files) pass the new checks unchanged. Reported by Kevin Backhouse (GitHub Security Lab). Co-Authored-By: Claude Fable 5.1 --- .github/CONTRIBUTING.md | 8 + .github/workflows/main.yml | 2 +- scripts/common/__init__.py | 0 scripts/common/provider_tree.py | 159 ++++++++++++++++++ ...pull-additional-docs-from-artifact-repo.py | 11 +- scripts/package/package-provider-docs.py | 11 +- scripts/package/sign-file.sh | 2 +- scripts/package/sign-provider-docs.py | 32 ++-- scripts/package/update-versions.py | 14 +- .../publish-provider-docs-to-artifact-repo.py | 9 +- scripts/setup-js/get-version.js | 24 ++- scripts/setup-js/setup-job.js | 58 +++++-- scripts/setup/get-updated-providers.py | 66 ++++++-- scripts/tests/simulate-REGISTRY-PULL.py | 20 ++- 14 files changed, 350 insertions(+), 66 deletions(-) create mode 100644 scripts/common/__init__.py create mode 100644 scripts/common/provider_tree.py diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index ec71840f..26942972 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -4,3 +4,11 @@ Please fork our repository and then raise a Pull Request (PR) against our `dev` branch. Please add a fulsome description in the Pull Request comment. We will attend to it and work with you as soon as possible. Once the PR is approved, we will sign any changed files, package for distribution and squash / merge the combined change set into the `dev` branch. Once merged into `dev`, changes will instantly be available to `stackql` applications configured to integrate the `dev` registry. After an additional period of monitoring from the team, it will be promoted to our main / production branch. This equates to publication for default configured instances of `stackql`. + +## Provider Document Requirements + +Provider documents are signed and published by CI after merge, so the pipeline enforces the layout below and rejects anything else: + +- documents live at `providers/src//v00.00.00000/provider.yaml` and `providers/src//v00.00.00000/services/.yaml`, nothing else is permitted under the version directory +- provider directory names and service file names must start with a letter, digit or `_` and contain only letters, digits, `.`, `_` and `-` +- every entry must be a regular file or directory; symbolic links are not permitted anywhere under `providers/src` diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 34a9a16d..bf29aa08 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -121,7 +121,7 @@ jobs: while IFS= read -r provider do echo "testing ${provider}..." - sh test-provider.sh $provider false $providersdir + sh test-provider.sh "$provider" false "$providersdir" done < ../providers.txt # diff --git a/scripts/common/__init__.py b/scripts/common/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/scripts/common/provider_tree.py b/scripts/common/provider_tree.py new file mode 100644 index 00000000..7c30989a --- /dev/null +++ b/scripts/common/provider_tree.py @@ -0,0 +1,159 @@ +""" +Shared validation helpers for the registry build pipeline. + +Everything under providers/src is contributor controlled: directory names, +file names and file types all arrive via pull requests, and the post merge +run of .github/workflows/main.yml signs and publishes that content with the +registry signing key in its environment. Before any contributor supplied +name is interpolated into a path, passed to a tool, signed, packaged or +written to $GITHUB_ENV it is checked here. +""" + +import os +import re +import stat +import sys +import uuid + +# provider dirs, version dirs, service files and artifact file names: must +# start with an alphanumeric character or '_' and contain only alphanumerics, +# '.', '_' and '-'. This rules out shell metacharacters, quotes, whitespace, +# path separators, '.' / '..', hidden files and leading '-'. +# (matched with fullmatch: unlike '$', it does not accept a trailing newline) +SAFE_NAME_RE = re.compile(r'[A-Za-z0-9_][A-Za-z0-9._-]*') + +# environment variable names written to $GITHUB_ENV +ENV_NAME_RE = re.compile(r'[A-Za-z_][A-Za-z0-9_]*') + +PROVIDERS_SRC_ROOT = os.path.join('providers', 'src') + + +def fail(message): + print("ERROR: %s" % (message), file=sys.stderr) + sys.exit(1) + + +def is_safe_name(name): + return isinstance(name, str) and SAFE_NAME_RE.fullmatch(name) is not None + + +def require_safe_name(name, what): + if not is_safe_name(name): + fail("invalid %s %r: must match %s" % (what, name, SAFE_NAME_RE.pattern)) + return name + + +def _is_within(path, root): + real_root = os.path.realpath(root) + real_path = os.path.realpath(path) + return real_path == real_root or real_path.startswith(real_root + os.sep) + + +def _lstat(path): + try: + return os.lstat(path) + except OSError as e: + fail("cannot stat %s: %s" % (path, e)) + + +def require_regular_file(path, root): + """path must be a regular file (not a symlink, device, fifo, ...) that + resolves to a location inside root""" + st = _lstat(path) + if stat.S_ISLNK(st.st_mode): + fail("%s is a symbolic link; symlinks are not permitted under %s" % (path, PROVIDERS_SRC_ROOT)) + if not stat.S_ISREG(st.st_mode): + fail("%s is not a regular file" % (path)) + if not _is_within(path, root): + fail("%s resolves outside %s" % (path, root)) + return path + + +def require_real_dir(path, root): + """path must be a directory (not a symlink to one) inside root""" + st = _lstat(path) + if stat.S_ISLNK(st.st_mode): + fail("%s is a symbolic link; symlinks are not permitted under %s" % (path, PROVIDERS_SRC_ROOT)) + if not stat.S_ISDIR(st.st_mode): + fail("%s is not a directory" % (path)) + if not _is_within(path, root): + fail("%s resolves outside %s" % (path, root)) + return path + + +def validate_provider_source_tree(provider_dir, source_version, repo_root='.'): + """ + Validate providers/src// on disk: + + providers/src///provider.yaml regular file + providers/src///services/ directory + providers/src///services/ regular files + + Every name is checked against SAFE_NAME_RE, no path component may be a + symlink, nothing may resolve outside the provider version directory and + no other entries are permitted. Returns the sorted list of service file + names. + """ + require_safe_name(provider_dir, 'provider directory name') + require_safe_name(source_version, 'provider version') + + # walk down one component at a time so a symlinked parent is caught too + providers_root = require_real_dir(os.path.join(repo_root, 'providers'), repo_root) + src_root = require_real_dir(os.path.join(providers_root, 'src'), providers_root) + provider_root = require_real_dir(os.path.join(src_root, provider_dir), src_root) + version_root = require_real_dir(os.path.join(provider_root, source_version), provider_root) + + require_regular_file(os.path.join(version_root, 'provider.yaml'), version_root) + + services_dir = require_real_dir(os.path.join(version_root, 'services'), version_root) + + service_files = sorted(os.listdir(services_dir)) + for service_file in service_files: + require_safe_name(service_file, 'service file name') + require_regular_file(os.path.join(services_dir, service_file), services_dir) + + for entry in os.listdir(version_root): + if entry not in ('provider.yaml', 'services'): + fail("unexpected entry %r in %s; only provider.yaml and services/ are permitted" % (entry, version_root)) + + return service_files + + +def parse_artifact_key(key, provider_path): + """ + Validate an artifact repo object key of the form + + // + + and return (provider_dir, file). Keys are joined onto local paths before + download, so anything else (extra or missing segments, '..', empty or + unsafe names) is rejected. + """ + prefix = provider_path.rstrip('/') + '/' + if not key.startswith(prefix): + fail("artifact key %r is not under %s" % (key, prefix)) + parts = key[len(prefix):].split('/') + if len(parts) != 2: + fail("artifact key %r does not have the form %s/" % (key, prefix)) + provider_dir = require_safe_name(parts[0], 'provider directory name') + file_name = require_safe_name(parts[1], 'artifact file name') + return provider_dir, file_name + + +def write_github_env(name, value): + """ + Append NAME=VALUE to $GITHUB_ENV using the heredoc form the runner + understands, the same way @actions/core exportVariable does. The value + is written directly to the file; nothing goes through a shell. + """ + env_file = os.getenv('GITHUB_ENV') + if not env_file: + fail("GITHUB_ENV is not set") + if not ENV_NAME_RE.fullmatch(name): + fail("invalid environment variable name %r" % (name)) + value = str(value) + delimiter = "ghadelimiter_%s" % (uuid.uuid4()) + if delimiter in value: + fail("unexpected delimiter collision writing %s" % (name)) + with open(env_file, 'a', encoding='utf-8') as f: + f.write("%s<<%s\n%s\n%s\n" % (name, delimiter, value, delimiter)) diff --git a/scripts/deploy/pull-additional-docs-from-artifact-repo.py b/scripts/deploy/pull-additional-docs-from-artifact-repo.py index 828d9ba4..04872bb3 100644 --- a/scripts/deploy/pull-additional-docs-from-artifact-repo.py +++ b/scripts/deploy/pull-additional-docs-from-artifact-repo.py @@ -3,9 +3,15 @@ from datetime import datetime, timedelta from dateutil.relativedelta import * +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +from common.provider_tree import parse_artifact_key, require_safe_name + print("getting REG_ARTIFACT_REPO_BUCKET env var...") repo_bucket_name = os.getenv('REG_ARTIFACT_REPO_BUCKET') +print("getting REG_PROVIDER_PATH env var...") +provider_path = os.getenv('REG_PROVIDER_PATH') + ## TODO: ## implement min, max and age rules @@ -37,7 +43,7 @@ updated_providers = [] print("getting updated providers...") for provider in providers: - updated_providers.append(provider['provider_dir']) + updated_providers.append(require_safe_name(provider['provider_dir'], 'provider directory name')) # # pull additional docs from artifact repo needed for deployment @@ -80,7 +86,8 @@ for req_file in req_files: print("pulling %s from artifact repo to [%s/%s]" % (req_file, os.getenv('REG_WEBSITE_DIR'), req_file)) - provider_dir = req_file.split('/')[-2] + # the object key is joined onto a local path below: accept only // + provider_dir, _ = parse_artifact_key(req_file, provider_path) print("creating dest dir for %s (if it doesn't exist)..." % (provider_dir)) os.makedirs("%s/%s/%s" % (os.getenv('REG_WEBSITE_DIR'), os.getenv('REG_PROVIDER_PATH'), provider_dir), exist_ok=True) s3_client.download_file(repo_bucket_name, req_file, "%s/%s" % (os.getenv('REG_WEBSITE_DIR'), req_file)) diff --git a/scripts/package/package-provider-docs.py b/scripts/package/package-provider-docs.py index bccf9bcd..7ec8a80f 100644 --- a/scripts/package/package-provider-docs.py +++ b/scripts/package/package-provider-docs.py @@ -1,5 +1,8 @@ import json, os, tarfile, subprocess, sys, shutil +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +from common.provider_tree import require_safe_name + def tardirectory(srcdir, arcname, dir2tar): p = subprocess.Popen(["tar", "-czf", arcname, dir2tar], stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=srcdir) p.wait() @@ -12,12 +15,12 @@ def tardirectory(srcdir, arcname, dir2tar): providers = json.loads(os.getenv('PROVIDERS')) print("getting REG_TARGET_BRANCH env var...") -target_branch = os.getenv('REG_TARGET_BRANCH') +target_branch = require_safe_name(os.getenv('REG_TARGET_BRANCH'), 'REG_TARGET_BRANCH') for provider in providers: provider_name = provider["provider"] - provider_dir = provider["provider_dir"] - version = provider["target_version"] + provider_dir = require_safe_name(provider["provider_dir"], 'provider directory name') + version = require_safe_name(provider["target_version"], 'target version') if target_branch == 'main': key = "%s.tgz" % (version) @@ -32,4 +35,4 @@ def tardirectory(srcdir, arcname, dir2tar): dest = "%s/%s/%s/%s" % (os.getenv('REG_WEBSITE_DIR'), os.getenv('REG_PROVIDER_PATH'), provider_dir, key) print("moving %s to %s" % (src, dest)) - shutil.move(src, dest) \ No newline at end of file + shutil.move(src, dest) diff --git a/scripts/package/sign-file.sh b/scripts/package/sign-file.sh index d590b3e2..6a5fbf78 100644 --- a/scripts/package/sign-file.sh +++ b/scripts/package/sign-file.sh @@ -5,4 +5,4 @@ outfile=$2 echo "Signing $infile..." -./ed25519tool sign --privatekeyenvvar=SIGNING_PRIV_KEY $infile -o $outfile +./ed25519tool sign --privatekeyenvvar=SIGNING_PRIV_KEY "$infile" -o "$outfile" diff --git a/scripts/package/sign-provider-docs.py b/scripts/package/sign-provider-docs.py index 300f0d6b..33a5a497 100644 --- a/scripts/package/sign-provider-docs.py +++ b/scripts/package/sign-provider-docs.py @@ -1,4 +1,7 @@ -import json, sys, os, subprocess +import json, sys, os, subprocess, shutil + +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +from common.provider_tree import require_safe_name, validate_provider_source_tree sign_file_script = "scripts/package/sign-file.sh" @@ -25,16 +28,21 @@ def sign_file(srcfile, tgtfile): # Add the signature to each provider for provider in providers: provider_name = provider["provider"] - provider_dir = provider["provider_dir"] - source_version = provider["source_version"] - target_version = provider["target_version"] - + provider_dir = require_safe_name(provider["provider_dir"], 'provider directory name') + source_version = require_safe_name(provider["source_version"], 'provider version') + target_version = require_safe_name(provider["target_version"], 'target version') + + # this step runs with the signing key in its environment: refuse symlinks, + # non-regular files, unexpected entries and unsafe names before any file + # under providers/src is opened + service_files = validate_provider_source_tree(provider_dir, source_version) + src_root_dir = "providers/src/%s/%s" % (provider_dir, source_version) src_services_dir = "%s/services" % (src_root_dir) - tgt_root_dir = "signed/providers/src/%s/%s" % (provider_dir, target_version) + tgt_root_dir = "signed/providers/src/%s/%s" % (provider_dir, target_version) tgt_services_dir = "%s/services" % (tgt_root_dir) - + if not os.path.exists(tgt_services_dir): os.makedirs(tgt_services_dir) @@ -42,13 +50,11 @@ def sign_file(srcfile, tgtfile): tgtfile = "%s/provider.yaml.sig" % (tgt_root_dir) sign_file(srcfile, tgtfile) - os.system("cp %s/provider.yaml %s/provider.yaml" % (src_root_dir, tgt_root_dir)) + shutil.copyfile(srcfile, "%s/provider.yaml" % (tgt_root_dir)) # sign each service - for service_file in os.listdir(src_services_dir): + for service_file in service_files: srcfile = "%s/%s" % (src_services_dir, service_file) tgtfile = "%s/%s.sig" % (tgt_services_dir, service_file) - sign_file(srcfile, tgtfile) - os.system("cp %s/%s %s/%s" % (src_services_dir, service_file, tgt_services_dir, service_file)) - - + sign_file(srcfile, tgtfile) + shutil.copyfile(srcfile, "%s/%s" % (tgt_services_dir, service_file)) diff --git a/scripts/package/update-versions.py b/scripts/package/update-versions.py index 33914893..bf4489eb 100644 --- a/scripts/package/update-versions.py +++ b/scripts/package/update-versions.py @@ -2,6 +2,9 @@ from fileinput import FileInput +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +from common.provider_tree import require_safe_name, validate_provider_source_tree + print("getting PROVIDERS env var...") providers = json.loads(os.getenv('PROVIDERS')) @@ -9,9 +12,13 @@ # update versions globally in the provider.yaml for each provider for provider in providers: provider_name = provider["provider"] - provider_dir = provider["provider_dir"] - source_version = provider["source_version"] - target_version = provider["target_version"] + provider_dir = require_safe_name(provider["provider_dir"], 'provider directory name') + source_version = require_safe_name(provider["source_version"], 'provider version') + target_version = require_safe_name(provider["target_version"], 'target version') + + # FileInput(inplace=True) follows symlinks; refuse anything that is not a + # regular file inside the provider tree before rewriting + validate_provider_source_tree(provider_dir, source_version) print("updating %s from %s to %s" % (provider_name, source_version, target_version)) @@ -21,4 +28,3 @@ for line in f: op = line.replace(source_version, target_version) print(op, end='') - \ No newline at end of file diff --git a/scripts/publish/publish-provider-docs-to-artifact-repo.py b/scripts/publish/publish-provider-docs-to-artifact-repo.py index c4c7be0f..f7516e5e 100644 --- a/scripts/publish/publish-provider-docs-to-artifact-repo.py +++ b/scripts/publish/publish-provider-docs-to-artifact-repo.py @@ -2,12 +2,15 @@ from botocore.exceptions import ClientError from boto3.dynamodb.conditions import Key, Attr +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +from common.provider_tree import require_safe_name + # # get env vars # print("getting REG_TARGET_BRANCH env var...") -target_branch = os.getenv('REG_TARGET_BRANCH') +target_branch = require_safe_name(os.getenv('REG_TARGET_BRANCH'), 'REG_TARGET_BRANCH') print("getting REG_SHA env var...") commit_sha = os.getenv('REG_SHA') @@ -82,8 +85,8 @@ def upload_file(file_name, bucket, object_name, provider, target_branch): print("getting updated providers...") for provider in providers: provider_name = provider['provider'] - provider_dir = provider['provider_dir'] - version = provider['target_version'] + provider_dir = require_safe_name(provider['provider_dir'], 'provider directory name') + version = require_safe_name(provider['target_version'], 'target version') print("processing %s (%s) version %s..." % (provider_name, target_branch, version)) diff --git a/scripts/setup-js/get-version.js b/scripts/setup-js/get-version.js index 826b14ce..3ad1a52e 100644 --- a/scripts/setup-js/get-version.js +++ b/scripts/setup-js/get-version.js @@ -1,7 +1,23 @@ -import { exec } from 'node:child_process'; +import * as core from '@actions/core'; -const version = `v${process.env['REG_COMMIT_YEAR']}.${process.env['REG_COMMIT_MONTH']}.${process.env['REG_PR_NO'].padStart(5, '0')}`; +function requireDigits(name, value) { + if (!/^\d+$/.test(String(value))) { + throw new Error(`${name} must be numeric, got: ${JSON.stringify(value)}`); + } + return String(value); +} -console.log(`REG_VERSION: ${version}`); +try { + const year = requireDigits('REG_COMMIT_YEAR', process.env['REG_COMMIT_YEAR']); + const month = requireDigits('REG_COMMIT_MONTH', process.env['REG_COMMIT_MONTH']); + const prNumber = requireDigits('REG_PR_NO', process.env['REG_PR_NO']); -exec(`echo "REG_VERSION=${version}" >> $GITHUB_ENV`); + const version = `v${year}.${month}.${prNumber.padStart(5, '0')}`; + + console.log(`REG_VERSION: ${version}`); + + // written to $GITHUB_ENV with a heredoc delimiter, not through a shell + core.exportVariable('REG_VERSION', version); +} catch (error) { + core.setFailed(error.message); +} diff --git a/scripts/setup-js/setup-job.js b/scripts/setup-js/setup-job.js index d032af8a..fd56c449 100644 --- a/scripts/setup-js/setup-job.js +++ b/scripts/setup-js/setup-job.js @@ -1,6 +1,20 @@ import * as core from '@actions/core'; import * as github from '@actions/github'; -import { exec } from 'node:child_process'; + +// subject line of a GitHub generated merge commit, e.g. +// Merge pull request #11 from stackql/feature/testing2 +// Merge pull request #11 from stackql/feature/testing2 [skip ci] +const MERGE_COMMIT_RE = /^Merge pull request #(\d+) from (\S+)/; +// subject line of a GitHub generated squash merge commit, e.g. +// some pull request title (#11) +const SQUASH_COMMIT_RE = /\(#(\d+)\)\s*$/; + +function requireDigits(name, value) { + if (!/^\d+$/.test(String(value))) { + throw new Error(`${name} must be numeric, got: ${JSON.stringify(value)}`); + } + return String(value); +} async function run() { try { @@ -26,24 +40,40 @@ async function run() { baseSha = context.payload.before; message = context.payload.head_commit.message.split('\n')[0]; console.log(`Commit Message: ${message}`); - // Merge pull request #11 from stackql/feature/testing2 - const commitMessageParts = message.split(' '); - prNumber = commitMessageParts[3].split('#')[1]; - sourceBranch = commitMessageParts[5].replace(`${context.payload.organization.login}/`, ''); targetBranch = context.payload.ref.replace('refs/heads/', ''); - // console.log(JSON.stringify(context, undefined, 2)); + const mergeMatch = MERGE_COMMIT_RE.exec(message); + const squashMatch = SQUASH_COMMIT_RE.exec(message); + if (mergeMatch) { + prNumber = mergeMatch[1]; + sourceBranch = mergeMatch[2]; + const ownerPrefix = `${context.repo.owner}/`; + if (sourceBranch.startsWith(ownerPrefix)) { + sourceBranch = sourceBranch.substring(ownerPrefix.length); + } + } else if (squashMatch) { + prNumber = squashMatch[1]; + sourceBranch = ''; + } else { + core.setFailed(`Unable to determine pull request number from commit message: ${message}`); + return; + } } else { core.setFailed(`Unsupported event: ${eventName}`); return; } - exec(`echo "REG_EVENT=${eventName}" >> $GITHUB_ENV`); - exec(`echo "REG_SHA=${shortSha}" >> $GITHUB_ENV`); - exec(`echo "REG_COMMIT_SHA=${commitSha}" >> $GITHUB_ENV`); - exec(`echo "REG_BASE_SHA=${baseSha}" >> $GITHUB_ENV`); - exec(`echo "REG_ACTION=${action}" >> $GITHUB_ENV`); - exec(`echo "REG_PR_NO=${prNumber}" >> $GITHUB_ENV`); - exec(`echo "REG_SOURCE_BRANCH=${sourceBranch}" >> $GITHUB_ENV`); - exec(`echo "REG_TARGET_BRANCH=${targetBranch}" >> $GITHUB_ENV`); + prNumber = requireDigits('pull request number', prNumber); + + // branch names, pull request titles and commit messages are contributor + // controlled. exportVariable appends to $GITHUB_ENV using a heredoc + // delimiter; nothing here is passed through a shell. + core.exportVariable('REG_EVENT', eventName); + core.exportVariable('REG_SHA', shortSha); + core.exportVariable('REG_COMMIT_SHA', commitSha); + core.exportVariable('REG_BASE_SHA', baseSha); + core.exportVariable('REG_ACTION', action); + core.exportVariable('REG_PR_NO', prNumber); + core.exportVariable('REG_SOURCE_BRANCH', sourceBranch); + core.exportVariable('REG_TARGET_BRANCH', targetBranch); } catch (error) { core.setFailed(error.message); return; diff --git a/scripts/setup/get-updated-providers.py b/scripts/setup/get-updated-providers.py index 206bf9d6..b914f309 100644 --- a/scripts/setup/get-updated-providers.py +++ b/scripts/setup/get-updated-providers.py @@ -1,8 +1,32 @@ import os, json, sys +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +from common.provider_tree import fail, require_safe_name, validate_provider_source_tree, write_github_env + print("getting REG_VERSION env var...") -target_version = os.getenv('REG_VERSION') +target_version = require_safe_name(os.getenv('REG_VERSION'), 'REG_VERSION') + +# +# paths in diff.txt come straight from the pull request author. only the +# following layouts are accepted, and every component must be a safe name +# (see scripts/common/provider_tree.py): +# +# providers/src///provider.yaml +# providers/src///services/ +# + +def parse_provider_path(path): + parts = path.split('/') + if len(parts) == 5 and parts[4] == 'provider.yaml': + pass + elif len(parts) == 6 and parts[4] == 'services': + require_safe_name(parts[5], 'service file name') + else: + fail("unexpected path under providers/src: %r (expected //provider.yaml or //services/)" % (path)) + provider_dir = require_safe_name(parts[2], 'provider directory name') + source_version = require_safe_name(parts[3], 'provider version') + return provider_dir, source_version print("finding updated providers...") @@ -11,17 +35,29 @@ updates = [] all_provider_versions = [] for line in lines: + line = line.rstrip('\r\n') + if not line.strip(): + continue fields = line.split('\t') action = fields[0] - path = fields[1] + # renames and copies (R/C) list the old and new path; the new path is the one on disk + path = fields[-1] + if path.startswith('"'): + # git C-quotes paths containing control, non-ASCII, quote or backslash + # characters; such names are never valid provider paths + if 'providers/src/' in path: + fail("provider path contains characters that are not permitted: %s" % (path)) + continue + if path in ('providers', 'providers/src'): + # only appears in a name-status diff if the directory was replaced by a file or symlink + fail("%s must be a directory" % (path)) if path.startswith('providers/src/'): provider = {} - provider_dir = path.split('/')[2] + provider_dir, source_version = parse_provider_path(path) if provider_dir == 'googleapis.com': provider_name = 'google' else: provider_name = provider_dir - source_version = path.split('/')[3] if source_version != 'v00.00.00000': print('ERROR: baseline version for providers must be v00.00.00000') sys.exit(1) @@ -31,7 +67,7 @@ provider['source_version'] = source_version provider['target_version'] = target_version provider['action'] = action - provider['path'] = path.rstrip('\n') + provider['path'] = path updates.append(provider) if provider_name == 'awscc': # add faux provider update for aws, as aws is a dependency of awscc @@ -53,14 +89,21 @@ print("%s providers updated" % (str(num_providers))) print("providers updated : %s" % (providers)) + # refuse symlinks, non-regular files, unexpected entries and unsafe names in + # the checked out tree now, before any later step reads it with secrets in + # the environment + for provider in providers: + print("validating providers/src/%s/%s..." % (provider['provider_dir'], provider['source_version'])) + validate_provider_source_tree(provider['provider_dir'], provider['source_version']) + if num_providers > 0: print("setting environment variables...") - + # write provider/version json to the PROVIDERS env var - os.system("echo '%s' >> $GITHUB_ENV" % ("PROVIDERS=" + json.dumps(providers))) + write_github_env('PROVIDERS', json.dumps(providers)) # populate NUM_PROVIDERS env var - os.system("echo ""%s"" >> $GITHUB_ENV" % ("NUM_PROVIDERS=" + str(num_providers))) + write_github_env('NUM_PROVIDERS', str(num_providers)) print("writing output files...") @@ -78,12 +121,11 @@ # write all provider updates to file with open('updates.json', 'w') as f: f.write(json.dumps(updates)) - + else: # write empty provider/version json to the PROVIDERS env var - os.system("echo '%s' >> $GITHUB_ENV" % ("PROVIDERS=" + json.dumps(providers))) + write_github_env('PROVIDERS', json.dumps(providers)) # set NUM_PROVIDERS env var == 0 - os.system("echo ""%s"" >> $GITHUB_ENV" % ("NUM_PROVIDERS=" + str(num_providers))) - + write_github_env('NUM_PROVIDERS', str(num_providers)) diff --git a/scripts/tests/simulate-REGISTRY-PULL.py b/scripts/tests/simulate-REGISTRY-PULL.py index 2e594bf2..866233b7 100644 --- a/scripts/tests/simulate-REGISTRY-PULL.py +++ b/scripts/tests/simulate-REGISTRY-PULL.py @@ -1,15 +1,18 @@ -import json, tarfile, os +import json, tarfile, os, sys + +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')) +from common.provider_tree import require_safe_name print("getting PROVIDERS env var...") providers = json.loads(os.getenv('PROVIDERS')) print("getting REG_TARGET_BRANCH env var...") -target_branch = os.getenv('REG_TARGET_BRANCH') +target_branch = require_safe_name(os.getenv('REG_TARGET_BRANCH'), 'REG_TARGET_BRANCH') for provider in providers: provider_name = provider["provider"] - provider_dir = provider["provider_dir"] - version = provider["target_version"] + provider_dir = require_safe_name(provider["provider_dir"], 'provider directory name') + version = require_safe_name(provider["target_version"], 'target version') if target_branch == 'main': key = "%s/%s/%s.tgz" % (os.getenv('REG_PROVIDER_PATH'), provider_dir, version) @@ -18,7 +21,8 @@ archive = "%s/%s" % (os.getenv('REG_WEBSITE_DIR'), key) print("extracting %s" % (archive)) - - file = tarfile.open(archive) - file.extractall("provider-tests/src/%s" % (provider_dir)) - file.close() + + # the 'data' filter (PEP 706) refuses absolute paths, path traversal, links + # pointing outside the destination and special files + with tarfile.open(archive) as file: + file.extractall("provider-tests/src/%s" % (provider_dir), filter='data')