Promote CI pipeline hardening to main (#470)

.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/<provider>/v00.00.00000/provider.yaml` and `providers/src/<provider>/v00.00.00000/services/<service>.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`

.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
 
       #

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/<provider_dir>/<source_version> on disk:
+
+        providers/src/<provider_dir>/<source_version>/provider.yaml    regular file
+        providers/src/<provider_dir>/<source_version>/services/        directory
+        providers/src/<provider_dir>/<source_version>/services/<svc>   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
+
+        <provider_path>/<provider_dir>/<file>
+
+    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<provider>/<file>" % (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))

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 <REG_PROVIDER_PATH>/<provider>/<file>
+    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))

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)
+    shutil.move(src, dest)

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"

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,30 +28,33 @@ 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)
 
     srcfile = "%s/provider.yaml" % (src_root_dir)
     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))

scripts/package/update-versions.py

@@ -2,16 +2,23 @@
 
 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'))
 
 # 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='')
-

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))
 

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);
+}