Phase 3 (part 1): harden the results tab and manifest#19
Merged
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe extension is rebranded for Azure DevOps, uses reduced scopes, removes an external script dependency, validates dialog URLs, and improves results pagination and error rendering. Modernization documentation records completed hardening work and remaining server-side signing and SDK migration tasks. ChangesAzure DevOps modernization
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
This was referenced Jul 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 3 of
MODERNIZATION.md, split into a focused, low-risk first part: security + correctness + UX fixes to the build-results tab and a manifest cleanup. It deliberately does not touch the/miniviewer or do the SDK/React port (see Deferred below).What changed
Results tab (
tb-build-info)unpkg.com/text-encoding<script>frominfoTab.html. It was a supply-chain risk on a credential-holding page and pure dead weight —window.TextDecoderis native in every browser Azure DevOps supports (its only use,info.js, already calls the native one).embedDialognow only frameshttps://testingbot.comURLs; anything else renders a message instead of loading an attacker-suppliable page. (dialog.js)info.jspagination/UX:count. This fixes corrupted page links/offsets after navigating to a partial last page.countis 0.build.orchestrationPlan.Manifest (3.6)
vso.build+vso.serviceendpoint_query(droppedvso.build_execute,vso.test,vso.test_write,vso.serviceendpoint_manage).Deferred / blocked (documented in
MODERNIZATION.md)/miniviewer needsmd5(key:secret:session_id), and the TestingBot public API exposes no endpoint that returns that share hash server-side (onlyPUT test[public], which returns nothing). So keeping/mini— which you asked to keep — keeps the secret in the build attachment until TestingBot adds a share-hash/URL endpoint. The tab is otherwise untouched.vss-web-extension-sdk→azure-devops-extension-sdk/ React port is large, and the old SDK keeps working indefinitely, so it's a separate later PR.Verification
npm run packagebuilds the vsix: nounpkgin the bundledinfoTab.html, and the compiled manifest carries the reduced scopes + new name.testingbot.com/subdomains over https; rejects http, other hosts,testingbot.com.evil.com,javascript:, undefined) and pagination math (correct page list with a partial last page highlighted;count=0skipped).These are browser/manifest changes I can't exercise outside a live Azure DevOps org. Smoke-test the results tab (attachment load + API call) with the packaged vsix before publishing, and note the reduced scopes force installed users to re-authorize on upgrade.
Summary by CodeRabbit
Bug Fixes
Updates