From cf8956ebebb5a47ac17629710db7fa1f5b616faf Mon Sep 17 00:00:00 2001
From: Jochen Delabie <jochen@testingbot.com>
Date: Fri, 17 Jul 2026 22:19:06 +0200
Subject: [PATCH] Phase 3 (part 1): harden the results tab and manifest

Security, correctness and UX fixes to the build-results tab, plus manifest
cleanup. Does not touch the /mini viewer or migrate the SDK (see below).

Tab (tb-build-info)
- Remove the SRI-less unpkg.com text-encoding script from infoTab.html (3.1);
  window.TextDecoder is native in every browser Azure DevOps supports.
- embedDialog: only frame https URLs on testingbot.com; anything else shows a
  message instead of loading an attacker-suppliable page (3.5, dialog.js).
- info.js pagination/UX (3.4): use a stable page size captured from the first
  response instead of the current page's count (fixed corrupted page links on a
  partial last page), guard against an infinite loop when count is 0, render an
  error state instead of leaving "LOADING" forever, wrap pagination clicks in
  try/catch, and guard build.orchestrationPlan.

Manifest (3.6)
- Rebrand "Visual Studio Team Services"/VSTS -> Azure DevOps.
- Cut scopes to the minimum actually used: vso.build + vso.serviceendpoint_query
  (drop vso.build_execute, vso.test, vso.test_write, vso.serviceendpoint_manage).

Deferred / blocked (documented in MODERNIZATION.md)
- Removing the API secret from the browser (finding #1) is blocked: /mini needs
  md5(key:secret:session_id) and the TestingBot public API exposes no endpoint
  to sign it server-side, so keeping the embedded viewer keeps the secret in the
  attachment. The vss-web-extension-sdk -> azure-devops-extension-sdk/React port
  is large and the old SDK still works, so it is a separate later PR.

Verified: npm run package builds the vsix (no unpkg in the bundled tab, scopes
and rebrand present in the compiled manifest); the URL allowlist and pagination
math were unit-checked (partial-last-page, count=0, subdomain-spoof, non-https).
NOTE: browser/manifest changes need a live-org smoke test before release, and the
reduced scopes force re-authorization on upgrade.
---
 MODERNIZATION.md                | 18 ++++++++++--
 tb-build-info/infoTab.html      |  1 -
 tb-build-info/scripts/dialog.js | 25 ++++++++++++++---
 tb-build-info/scripts/info.js   | 49 +++++++++++++++++++++++----------
 vss-extension.json              |  6 ++--
 5 files changed, 74 insertions(+), 25 deletions(-)

diff --git a/MODERNIZATION.md b/MODERNIZATION.md
index 72ec65c..0f630b0 100644
--- a/MODERNIZATION.md
+++ b/MODERNIZATION.md
@@ -10,11 +10,25 @@ _Date: 2026-07-17. Applies to repo `vso-testingbot-plugin` (marketplace: `testin
 >   drift resolved via `scripts/stamp-version.js`, immediate correctness bugs fixed.
 > - **Phase 2 — DONE (PR #18):** tasks rewritten in TypeScript, runtime tunnel via
 >   `testingbot-tunnel-launcher` (jar deleted), secret pipeline variables, tests.
-> - **Phase 3 — PENDING:** results-tab / SDK rewrite, including moving the `/mini`
->   URL signing server-side so the secret leaves the browser (finding #1).
+> - **Phase 3 — PARTIAL (PR #19):** results-tab hardening — unpkg.com CDN script
+>   removed, pagination + error-state bugs fixed, `embedDialog` URL allowlisted;
+>   manifest rebranded and scopes cut to `vso.build` + `vso.serviceendpoint_query`.
+>   **Still PENDING:** (a) moving the `/mini` share-URL signing server-side so the
+>   secret leaves the browser (finding #1) — **blocked**: the TestingBot public
+>   API exposes no endpoint that returns the `md5(key:secret:session_id)` share
+>   hash, so keeping the embedded `/mini` viewer requires the secret in the
+>   browser until TestingBot adds such an endpoint; (b) the `vss-web-extension-sdk`
+>   → `azure-devops-extension-sdk` / React port (large, and the old SDK keeps
+>   working, so deferred to its own PR).
 > - **Phase 4 — PENDING:** CI/CD, lint, publishing.
 >
 > Items marked "PENDING" below are the genuinely outstanding work.
+>
+> **Release note for PR #19:** the scope reduction and tab changes are
+> browser/manifest-side and cannot be exercised outside a live Azure DevOps org.
+> Smoke-test the results tab (attachment load + API call) with the packaged vsix
+> before publishing; the reduced scopes force installed users to re-authorize on
+> upgrade.
 
 ## 1. Executive summary
 
diff --git a/tb-build-info/infoTab.html b/tb-build-info/infoTab.html
index e7e303f..f8ada21 100644
--- a/tb-build-info/infoTab.html
+++ b/tb-build-info/infoTab.html
@@ -2,7 +2,6 @@
 <head>
   <title>TestingBot Info</title>
   <script src="../lib/VSS.SDK.js"></script>
-  <script src="https://unpkg.com/text-encoding@0.6.1/lib/encoding.js"></script>
   <script type="text/javascript">
     VSS.init({ usePlatformScripts: true, moduleLoaderConfig: { paths: { "scripts": "tb-build-info/scripts" } } });
     VSS.ready(function() {
diff --git a/tb-build-info/scripts/dialog.js b/tb-build-info/scripts/dialog.js
index f93eb78..401d37b 100644
--- a/tb-build-info/scripts/dialog.js
+++ b/tb-build-info/scripts/dialog.js
@@ -13,8 +13,25 @@ const getParams = query => {
       return params;
     }, { });
 };
+// Only ever frame an https TestingBot URL. The url comes in as a query
+// parameter, so validate it before assigning it as the iframe source to avoid
+// framing an attacker-supplied page.
+const isAllowedUrl = value => {
+  try {
+    const parsed = new URL(value);
+    return parsed.protocol === 'https:' &&
+      (parsed.hostname === 'testingbot.com' || parsed.hostname.endsWith('.testingbot.com'));
+  } catch (err) {
+    return false;
+  }
+};
+
 const params = getParams(window.location.search);
-var iframe = document.createElement('iframe');
-iframe.src = params.url;
-iframe.style = 'width: 100%; min-height: 100vh; border: 0';
-document.body.appendChild(iframe);
\ No newline at end of file
+if (isAllowedUrl(params.url)) {
+  var iframe = document.createElement('iframe');
+  iframe.src = params.url;
+  iframe.style = 'width: 100%; min-height: 100vh; border: 0';
+  document.body.appendChild(iframe);
+} else {
+  document.body.textContent = 'Unable to display this test: the requested URL is not a valid TestingBot address.';
+}
\ No newline at end of file
diff --git a/tb-build-info/scripts/info.js b/tb-build-info/scripts/info.js
index f7e6f3a..1e4d186 100644
--- a/tb-build-info/scripts/info.js
+++ b/tb-build-info/scripts/info.js
@@ -213,30 +213,41 @@ const getBuildResponse = async function (buildInformation, offset) {
   return JSON.parse(json.result[0]);
 }
 
-const renderResults = function (buildInformation, buildFullJobs, buildFullMeta, currentPage) {
+const renderError = function (message) {
+  const $buildinfo = $('.build-info').empty();
+  $buildinfo.append($('<h2>').text(message));
+};
+
+// pageSize is the stable number of results per API page, captured once from the
+// first (offset 0) response. Using the *current* page's count as the divisor
+// corrupts the page links on a partial last page, and a count of 0 would make
+// the pagination loop spin forever.
+const renderResults = function (buildInformation, buildFullJobs, buildFullMeta, currentOffset, pageSize) {
   const $table = $('<table>');
   $table.css('min-width', '800px');
   $table.append('<thead><tr><th align="left">Test Name</th><th align="left">OS/Browser</th><th align="left">Pass/Fail</th></tr></thead>');
 
   const $footer = $('<ul style="padding-left: 0">');
-  if (buildFullMeta && buildFullMeta.count < buildFullMeta.total) {
-    let paginationCount = buildFullMeta.count
-    for (let i = 0; i < buildFullMeta.total; i += paginationCount) {
-      if (i === currentPage) {
-        $footer.append($('<li style="display: inline; margin-right: 8px">' + ((i + paginationCount) / paginationCount) + '</li>'))
+  if (pageSize > 0 && buildFullMeta && buildFullMeta.total > pageSize) {
+    for (let offset = 0; offset < buildFullMeta.total; offset += pageSize) {
+      const pageNumber = (offset / pageSize) + 1;
+      if (offset === currentOffset) {
+        $footer.append($('<li style="display: inline; margin-right: 8px">').text(pageNumber));
         continue;
       }
       $footer.append($('<li style="display: inline; margin-right: 8px">')
         .append($('<a>')
         .attr('href', '#')
-        .text((i + paginationCount) / paginationCount)
+        .text(pageNumber)
         .click(async function(e) {
           e.preventDefault();
-          const buildFullResponse = await getBuildResponse(buildInformation, i);
-          const buildFullJobs = buildFullResponse.data;
-          const buildFullMeta = buildFullResponse.meta;
-
-          renderResults(buildInformation, buildFullJobs, buildFullMeta, i);
+          try {
+            const buildFullResponse = await getBuildResponse(buildInformation, offset);
+            renderResults(buildInformation, buildFullResponse.data, buildFullResponse.meta, offset, pageSize);
+          } catch (err) {
+            console.error('error loading page', err);
+            renderError('Could not load TestingBot results for this page.');
+          }
         })));
     }
   }
@@ -278,6 +289,11 @@ const renderResults = function (buildInformation, buildFullJobs, buildFullMeta,
 
 sharedConfig.onBuildChanged(async function(build) {
   try {
+    if (!build || !build.orchestrationPlan) {
+      renderError('No test results found');
+      return;
+    }
+
     const taskAttachments = await taskRestClient.getPlanAttachments(
       webContext.project.id,
       'build',
@@ -286,8 +302,7 @@ sharedConfig.onBuildChanged(async function(build) {
     );
 
     if (taskAttachments.length < 1) {
-      const $buildinfo = $('.build-info').empty();
-      $buildinfo.append('<h2>No test results found</h2>');
+      renderError('No test results found');
       return;
     }
     const buildInformation = await taskRestClient.getAttachmentContent(
@@ -306,8 +321,12 @@ sharedConfig.onBuildChanged(async function(build) {
     const buildFullJobs = buildFullResponse.data;
     const buildFullMeta = buildFullResponse.meta;
 
-    renderResults(buildInformation, buildFullJobs, buildFullMeta, 0)
+    // Capture the page size once, from the first response, and thread it through
+    // so pagination stays stable regardless of the current page's count.
+    const pageSize = buildFullMeta ? buildFullMeta.count : 0;
+    renderResults(buildInformation, buildFullJobs, buildFullMeta, 0, pageSize)
   } catch (err) {
     console.error('error', err);
+    renderError('Could not load TestingBot results.');
   }
 });
\ No newline at end of file
diff --git a/vss-extension.json b/vss-extension.json
index 64bd912..fa7158e 100644
--- a/vss-extension.json
+++ b/vss-extension.json
@@ -1,11 +1,11 @@
 {
   "manifestVersion": 1,
   "id": "testingbot-tasks",
-  "name": "TestingBot for Visual Studio Team Services",
+  "name": "TestingBot for Azure DevOps",
   "version": "0.1.0",
   "publisher": "testingbot",
   "public": true,
-  "description": "Integrate TestingBot's Automated Testing with VSTS",
+  "description": "Integrate TestingBot's automated testing with Azure DevOps",
   "categories": ["Azure Pipelines", "Azure Test Plans"],
   "content": {
     "details": {
@@ -32,7 +32,7 @@
     { "path": "lib", "addressable": true }
 
   ],
-  "scopes": [ "vso.build_execute", "vso.test", "vso.test_write", "vso.serviceendpoint_manage" ],
+  "scopes": [ "vso.build", "vso.serviceendpoint_query" ],
   "targets": [
       {
         "id": "Microsoft.VisualStudio.Services"