Update CHANGELOG.md (#18599)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

.github/workflows/pr-script-test.yml

@@ -5,11 +5,15 @@ name: PR script test
 # date.
 #
 # A self-hosted runner that executes pull request code as root is only
-# acceptable if a human decides what runs on it. Nothing here starts on a push:
-# someone with write access comments `/test` (optionally followed by a full SHA
-# to pin, and the ct/ paths to build). Only a commit that existed when the
-# comment was written gets built: a push after it is never covered, not even
-# one that lands in the seconds before this job runs.
+# acceptable if a human decides what runs on it. So it starts on a push only
+# for trusted PRs: those the push-app-to-main app opens on a branch of this
+# repository when a script moves over from DevScripts (already reviewed and
+# tested there; only writers can push to such a branch). Those are built
+# automatically and approved when every script passes. Everything else waits
+# for someone with write access to comment `/test` (optionally followed by a
+# full SHA to pin, and the ct/ paths to build). Only a commit that existed when
+# the comment was written gets built: a push after it is never covered, not
+# even one that lands in the seconds before this job runs.
 #
 #   gate    ubuntu-latest   who asked, which scripts, which commit
 #   test    proxmox-test    build + checks, one matrix job per script/OS
@@ -24,6 +28,13 @@ name: PR script test
 on:
   issue_comment:
     types: [created]
+  # Trusted PRs only, see above. pull_request_target runs this file from main,
+  # never the PR's copy of it.
+  pull_request_target:
+    types: [opened, reopened, synchronize, ready_for_review]
+    paths:
+      - "ct/**"
+      - "install/**"
   workflow_dispatch:
     inputs:
       pr:
@@ -45,7 +56,10 @@ jobs:
   gate:
     if: >-
       github.event_name == 'workflow_dispatch' ||
-      (github.event.issue.pull_request && startsWith(github.event.comment.body, '/test'))
+      (github.event_name == 'issue_comment' && github.event.issue.pull_request && startsWith(github.event.comment.body, '/test')) ||
+      (github.event_name == 'pull_request_target' && !github.event.pull_request.draft &&
+       github.event.pull_request.user.login == 'push-app-to-main[bot]' &&
+       github.event.pull_request.head.repo.full_name == github.repository)
     runs-on: ubuntu-latest
     permissions:
       actions: read # when the run was dispatched, for a /test without SHA
@@ -60,6 +74,8 @@ jobs:
       sha: ${{ steps.plan.outputs.sha }}
       check_id: ${{ steps.plan.outputs.check_id }}
       actor: ${{ steps.plan.outputs.actor }}
+      not_built: ${{ steps.plan.outputs.not_built }}
+      trusted: ${{ steps.plan.outputs.trusted }}
     steps:
       - id: plan
         uses: actions/github-script@v8
@@ -69,8 +85,20 @@ jobs:
             const MAX = 10;
             core.setOutput('matrix', '[]');
 
+            // Trusted: a PR the push-app-to-main app opened on a branch of this
+            // repository, i.e. a script moving over from DevScripts.
+            const TRUSTED_AUTHOR = 'push-app-to-main[bot]';
+            const isTrusted = p => !!p && p.user && p.user.login === TRUSTED_AUTHOR && p.user.type === 'Bot' &&
+              !!p.head.repo && p.head.repo.full_name === `${owner}/${repo}`;
+
             let prNumber, requested = '';
-            if (context.eventName === 'workflow_dispatch') {
+            if (context.eventName === 'pull_request_target') {
+              const p = context.payload.pull_request;
+              if (!isTrusted(p)) return;
+              prNumber = p.number;
+              // Exactly the pushed commit; a later push starts its own run.
+              requested = p.head.sha;
+            } else if (context.eventName === 'workflow_dispatch') {
               prNumber = Number(context.payload.inputs.pr);
               requested = `${context.payload.inputs.sha || ''} ${context.payload.inputs.scripts || ''}`;
             } else {
@@ -93,6 +121,11 @@ jobs:
             const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: prNumber });
             if (pr.state !== 'open' || !pr.head.repo) { core.notice('PR is closed or its fork is gone'); return; }
             const head = pr.head.repo;
+            if (context.eventName === 'pull_request_target' && pr.head.sha !== requested) {
+              core.notice('Pushed to again since; the newer push has its own run');
+              return;
+            }
+            core.setOutput('trusted', isTrusted(pr) ? 'true' : 'false');
             const MARK = '<!-- pr-script-test -->';
             const sticky = async body => {
               const comments = await github.paginate(github.rest.issues.listComments, { owner, repo, issue_number: prNumber });
@@ -150,6 +183,10 @@ jobs:
               core.setFailed(`Only ct/<name>.sh paths can be requested, got: ${requested}`);
               return;
             }
+            // Everything the PR changes that this test cannot build is named in
+            // the comment, so a /test never ends in silence.
+            const notBuilt = [];
+            const code = s => '`' + String(s).replace(/[`\r\n]/g, '') + '`';
             if (entries.length === 0) {
               const files = await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number: prNumber });
               const set = new Set();
@@ -158,6 +195,8 @@ jobs:
                 let m;
                 if ((m = f.filename.match(/^install\/([A-Za-z0-9._-]+)-install\.sh$/))) set.add(`ct/${m[1]}.sh`);
                 else if (/^ct\/[A-Za-z0-9._-]+\.sh$/.test(f.filename)) set.add(f.filename);
+                else if (/^vm\/.*\.sh$/.test(f.filename)) notBuilt.push(`${code(f.filename)}: \`vm/\` scripts are not covered`);
+                else if (/^(tools|turnkey)\/.*\.sh$/.test(f.filename)) notBuilt.push(`${code(f.filename)}: only \`ct/\` scripts are built`);
               }
               entries = [...set].sort();
             }
@@ -168,14 +207,25 @@ jobs:
               try {
                 const res = await github.rest.repos.getContent({ owner: head.owner.login, repo: head.name, path: script, ref: pr.head.sha });
                 text = Buffer.from(res.data.content || '', 'base64').toString('utf8');
-              } catch (e) { continue; }   // install script without a ct/ entry point
+              } catch (e) {
+                notBuilt.push(`\`${script}\` does not exist in this PR (an \`install/\` script is built through the \`ct/\` script of the same name)`);
+                continue;
+              }
               // Scripts that ask for the OS get built once per OS.
               const oses = /msg_menu\s+"Choose the container OS"/.test(text) ? ['debian', 'alpine'] : [''];
               const name = script.slice(3, -3);
               for (const os of oses) include.push({ script, os, name: os ? `${name}-${os}` : name });
             }
-            if (entries.length > MAX) core.warning(`Only the first ${MAX} of ${entries.length} scripts are built`);
-            if (include.length === 0) { core.notice('No ct/ script to build'); return; }
+            for (const script of entries.slice(MAX)) notBuilt.push(`\`${script}\`: at most ${MAX} scripts per run, name it with \`/test ${script}\``);
+            if (include.length === 0) {
+              await sticky([
+                MARK, '### Proxmox script test', '',
+                'ℹ️ Nothing to build: this test builds `ct/` scripts (and changed `install/` scripts through their `ct/` script).', '',
+                ...(notBuilt.length ? notBuilt.map(n => `- ${n}`) : ['- this PR changes no `ct/` or `install/` script']),
+              ].join('\n'));
+              core.notice('No ct/ script to build');
+              return;
+            }
 
             const runUrl = `${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`;
             const { data: check } = await github.rest.checks.create({
@@ -189,6 +239,7 @@ jobs:
             core.setOutput('repo', head.full_name);
             core.setOutput('sha', pr.head.sha);
             core.setOutput('check_id', String(check.id));
+            core.setOutput('not_built', JSON.stringify(notBuilt));
             // Who asked: the /test commenter, or whoever dispatched the run.
             core.setOutput('actor', context.actor);
 
@@ -196,6 +247,7 @@ jobs:
               MARK, '### Proxmox script test', '',
               `⏳ Building ${include.length} at ${pr.head.sha.slice(0, 7)} — [watch it live](${runUrl}) (open the \`test\` job)`, '',
               ...include.map(i => `- \`${i.script}\`${i.os ? ` (${i.os})` : ''}`),
+              ...(notBuilt.length ? ['', '**Not built:**', ...notBuilt.map(n => `- ${n}`)] : []),
             ].join('\n'));
 
   test:
@@ -262,6 +314,8 @@ jobs:
           CHECK_ID: ${{ needs.gate.outputs.check_id }}
           TEST_RESULT: ${{ needs.test.result }}
           ACTOR: ${{ needs.gate.outputs.actor }}
+          NOT_BUILT: ${{ needs.gate.outputs.not_built }}
+          TRUSTED: ${{ needs.gate.outputs.trusted }}
         with:
           script: |
             const fs = require('fs');
@@ -415,6 +469,7 @@ jobs:
               `${headline} at \`${process.env.SHA.slice(0, 7)}\``, '',
               table, '',
               ...shots, '',
+              ...(() => { try { const n = JSON.parse(process.env.NOT_BUILT || '[]'); return n.length ? ['**Not built:**', ...n.map(x => `- ${x}`), ''] : []; } catch (e) { return []; } })(),
               ...logs, '',
               `Full logs: [workflow run](${runUrl}) (artifacts \`result-*\`). Comment \`/test\` to build again.`,
             ].join('\n');
@@ -452,14 +507,55 @@ jobs:
             const REVIEW_MARK = '<!-- pr-script-test-review -->';
             const failedRows = cancelled ? [] : rows.filter(r => r.status === 'fail');
             const sha7 = process.env.SHA.slice(0, 7);
+            // Trusted PRs (scripts moving over from DevScripts) are approved when
+            // every script passes -- not on untestable or setup trouble, which
+            // means "not tested", not "good". That approval counts toward the
+            // ruleset's required review; a push dismisses it as stale, and a
+            // later run that does not fully pass withdraws it.
+            // Anyone with write access can push to a migration branch, and the
+            // approval counts toward the required review. So it only covers
+            // what a migration contains: ct/ and install/ scripts. A PR that
+            // also changes anything else (workflows, misc/, ...) is tested but
+            // never approved.
+            let approve = process.env.TRUSTED === 'true' && allPass;
+            let noApproveWhy = '';
+            if (approve) {
+              const changed = await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number: pr });
+              const other = changed.map(f => f.filename).filter(n => !/^(ct|install)\/[A-Za-z0-9._-]+\.sh$/.test(n));
+              if (other.length) {
+                approve = false;
+                noApproveWhy = `Not approved automatically: besides \`ct/\` and \`install/\` scripts this PR changes ${other.slice(0, 5).map(n => '`' + n.replace(/[`\r\n]/g, '') + '`').join(', ')}${other.length > 5 ? ', …' : ''}. That needs a human review.`;
+              }
+            }
+            if (noApproveWhy) {
+              await github.rest.issues.updateComment({ owner, repo, comment_id: posted.id, body: `${body}\n\n> ${noApproveWhy}`.slice(0, 65000) });
+            }
             try {
               const reviews = await github.paginate(github.rest.pulls.listReviews, { owner, repo, pull_number: pr });
-              for (const v of reviews.filter(v => v.user && v.user.type === 'Bot' && v.state === 'CHANGES_REQUESTED' && (v.body || '').includes(REVIEW_MARK))) {
+              const ours = state => reviews.filter(v => v.user && v.user.type === 'Bot' && v.state === state && (v.body || '').includes(REVIEW_MARK));
+              for (const v of ours('CHANGES_REQUESTED')) {
                 await github.rest.pulls.dismissReview({
                   owner, repo, pull_number: pr, review_id: v.id,
                   message: failedRows.length ? 'Replaced by a newer Proxmox script test.' : `Proxmox script test passed at ${sha7}.`,
                 });
               }
+              if (!approve && !cancelled) {
+                for (const v of ours('APPROVED')) {
+                  await github.rest.pulls.dismissReview({
+                    owner, repo, pull_number: pr, review_id: v.id,
+                    message: `Withdrawn: the Proxmox script test at ${sha7} did not fully pass.`,
+                  });
+                }
+              }
+              if (approve && !ours('APPROVED').some(v => v.commit_id === process.env.SHA)) {
+                await github.rest.pulls.createReview({
+                  owner, repo, pull_number: pr, commit_id: process.env.SHA, event: 'APPROVE',
+                  body: [
+                    REVIEW_MARK, `**Proxmox script test passed** at \`${sha7}\`: ${rows.map(r => `\`${r.script}\``).join(', ')}.`, '',
+                    `Script moved over from DevScripts, built and checked on a real Proxmox host. [Test result](${posted.html_url}).`,
+                  ].join('\n'),
+                });
+              }
               if (failedRows.length) {
                 await github.rest.pulls.createReview({
                   owner, repo, pull_number: pr, commit_id: process.env.SHA, event: 'REQUEST_CHANGES',

.github/workflows/scripts/pve-run-script.sh

@@ -283,7 +283,13 @@ ci_value() {
   *pass* | *secret* | *token* | *key*) head -c 32 /dev/urandom | base64 | tr -dc 'A-Za-z0-9' | cut -c1-20 ;;
   *email* | *mail*) echo "ci@example.test" ;;
   *url* | *endpoint*) echo "https://ci-${host_app}.example.test" ;;
-  *host* | *domain* | *fqdn*) echo "ci-${host_app}.example.test" ;;
+  # One name per variable: var_opencloud_collabora_fqdn ->
+  # ci-opencloud-collabora.example.test, so several of them cannot collide.
+  *host* | *domain* | *fqdn*)
+    local part="${1#var_}"
+    part=$(sed -E 's/_?(fqdn|hostname|host|domain)$//' <<<"${part,,}" | tr '_' '-')
+    echo "ci-${part:-$host_app}.example.test"
+    ;;
   *user*) echo "ciadmin" ;;
   *) echo "ci-test" ;;
   esac
@@ -296,6 +302,29 @@ if [[ -f "$json" ]]; then
   done < <(jq -r '(if type == "array" then .[0] else . end).app_vars // [] | .[] |
     select(.name | test("^var_[a-z0-9_]{1,40}$")) | [.name, (.default // "" | tostring)] | @tsv' "$json" 2>/dev/null)
 fi
+# Names built at run time from a loop, e.g. OpenCloud:
+#   servers=("opencloud" "collabora" "wopi")
+#   for server in "${servers[@]}"; do fqdn_var="var_opencloud_${server}_fqdn"
+# Each var_...${x}... template is expanded with the words of `for x in ...`
+# (or of the array that loop walks). Nothing is executed, only read.
+expand_templates() {
+  local tpl x pre suf words arr w
+  while read -r tpl; do
+    x=$(sed -E 's/^[^$]*\$\{?([A-Za-z_][A-Za-z0-9_]*)\}?.*$/\1/' <<<"$tpl")
+    pre="${tpl%%\$*}"
+    suf=$(sed -E 's/^[^$]*\$\{?[A-Za-z_][A-Za-z0-9_]*\}?//' <<<"$tpl")
+    words=$(grep -m1 -oE "for ${x} in [^;]*" "$1" | sed -E "s/^for ${x} in //")
+    if [[ "$words" =~ \$\{([A-Za-z_][A-Za-z0-9_]*)\[@\]\} ]]; then
+      arr="${BASH_REMATCH[1]}"
+      words=$(grep -m1 -oE "^\s*${arr}=\([^)]*\)" "$1" | sed -E "s/^\s*${arr}=\(//; s/\)$//")
+    fi
+    for w in $words; do
+      w="${w//[\"\']/}"
+      [[ "${pre}${w}${suf}" =~ ^var_[a-z0-9_]{1,40}$ ]] && echo "${pre}${w}${suf}"
+    done
+  done < <(grep -oE 'var_[a-z0-9_]*\$\{?[A-Za-z_][A-Za-z0-9_]*\}?[a-z0-9_]*' "$1" | sort -u)
+}
+
 # The engine names the install script after APP (lower case, no spaces), which
 # need not match the ct/ file name: ct/bluesky-pds.sh -> blueskypds-install.sh.
 nsapp=$(grep -m1 -oE '^APP="[^"]+"' "$tree/$SCRIPT" | cut -d'"' -f2 | tr -d ' ' | tr '[:upper:]' '[:lower:]')
@@ -303,7 +332,10 @@ for f in "$tree/install/${app}-install.sh" "$tree/install/${nsapp}-install.sh" "
   [[ -f "$f" ]] || continue
   while read -r name; do
     [[ -v "answers[$name]" ]] || answers[$name]=""
-  done < <(grep -E '\bread\b|prompt_(input|input_required|confirm|select|password)\b' "$f" | grep -oE '\bvar_[a-z0-9_]{1,40}\b' | sort -u)
+  done < <({
+    grep -E '\bread\b|prompt_(input|input_required|confirm|select|password)\b' "$f" | grep -oE '\bvar_[a-z0-9_]{1,40}\b'
+    expand_templates "$f"
+  } | sort -u)
 done
 for name in "${!answers[@]}"; do
   [[ "$ENGINE_VARS" == *" $name "* ]] && continue

CHANGELOG.md

@@ -594,7 +594,13 @@ Exercise vigilance regarding copycat or coat-tailing sites that seek to exploit
 
 ### 💾 Core
 
-  - fix: flush the image before importing a VM disk [@michelroegl-brunner](https://github.com/michelroegl-brunner) ([core#223](https://github.com/community-scripts/core/pull/223))
+  - fix: keep a script's config file when a package installs it [@MickLesk](https://github.com/MickLesk) ([core#226](https://github.com/community-scripts/core/pull/226))
+- fix: flush the image before importing a VM disk [@michelroegl-brunner](https://github.com/michelroegl-brunner) ([core#223](https://github.com/community-scripts/core/pull/223))
+
+### 📂 Github
+
+  - pr-script-test: pre-answer prompt variables built in a loop [@michelroegl-brunner](https://github.com/michelroegl-brunner) ([#18596](https://github.com/community-scripts/ProxmoxVE/pull/18596))
+- Build PR scripts on a Proxmox test host via /test [@michelroegl-brunner](https://github.com/michelroegl-brunner) ([#18594](https://github.com/community-scripts/ProxmoxVE/pull/18594))
 
 ## 2026-11-09