From 79142b2dfaf00e26f329a9f0795703554a783bd8 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 10 Jun 2026 18:03:18 +0530 Subject: [PATCH 1/3] fix(accessibility): patch prototype command for class-based Nightwatch commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commandWrapper() patched `originalCommand.command` directly, which only works for web-element commands that export a plain object (`module.exports.command = fn`). executeScript/executeAsyncScript (and the protocol/client/appium commands) export a class with `command` on the prototype, so the patch landed on a non-existent static method and the real prototype command Nightwatch invokes stayed unpatched. As a result `performScan` never ran for `browser.execute('mobile:scroll', ...)` (and all other class-based commands listed in commands.json) on App Accessibility sessions — no accessibility scan was captured for those interactions. Detect whether `command` lives as an own property (object export) or on the prototype (class export) and patch the correct target, mirroring how the `protocolAction` branch already handles class-based commands. The existing `shouldPatchExecuteScript` recursion guard now becomes effective and prevents the plugin's own `browserstack_executor` scan scripts from re-triggering a scan. Verified on a real-device Android App Accessibility session: `mobile:scroll` now triggers exactly one performScan, with no recursion, and the test passes. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/accessibilityAutomation.js | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/accessibilityAutomation.js b/src/accessibilityAutomation.js index 7e914eb..c5816e5 100644 --- a/src/accessibilityAutomation.js +++ b/src/accessibilityAutomation.js @@ -437,9 +437,26 @@ class AccessibilityAutomation { try { const webElementCommandPath = path.join(nightwatchDir, `${commandJson[commandKey].path}`, `${commandName}.js`); const originalCommand = require(webElementCommandPath); - const originalCommandFn = originalCommand.command; - originalCommand.command = async function(...args) { + // Nightwatch commands are exported in two shapes: web-element commands + // export a plain object with an own `command` function, while client-commands, + // protocol and document commands (e.g. executeScript) export a class with + // `command` on the prototype. Patch wherever the command function actually lives. + const commandTarget = typeof originalCommand.command === 'function' + ? originalCommand + : (originalCommand.prototype && typeof originalCommand.prototype.command === 'function' + ? originalCommand.prototype + : null); + + if (!commandTarget) { + Logger.debug(`Failed to patch command ${commandName}: no command function found`); + + return; + } + + const originalCommandFn = commandTarget.command; + + commandTarget.command = async function(...args) { if ( !commandName.includes('execute') || !accessibilityInstance.shouldPatchExecuteScript(args.length ? args[0] : null) From 10352c4eec97de25b3f7efdb5b6d23abf3c26ba5 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 10 Jun 2026 18:25:26 +0530 Subject: [PATCH 2/3] fix(accessibility): scan function-form execute/executeAsyncScript user scripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit shouldPatchExecuteScript() treated any non-string script as a script to skip (`!script || typeof script !== 'string'` -> return true), so user scripts passed as functions — `browser.execute(function(){...})` / `executeAsyncScript(fn)` — never triggered performScan, even though they can mutate page/screen state just like a string script. The plugin's own scan scripts are always strings carrying the `browserstack_executor` token (verified: every internal execute* call in the SDK passes a string), so a non-string script is always a user script and is safe to scan — the string-based recursion guard is unaffected. Treat an empty/undefined script as skip (unchanged) and a function script as a user script to scan. Verified on a real-device Android App Accessibility session: executeAsyncScript(fn) now triggers exactly one performScan; the 18 internal browserstack_executor scan scripts are still skipped (no recursion); test passes. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/accessibilityAutomation.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/accessibilityAutomation.js b/src/accessibilityAutomation.js index c5816e5..67b24f3 100644 --- a/src/accessibilityAutomation.js +++ b/src/accessibilityAutomation.js @@ -475,10 +475,17 @@ class AccessibilityAutomation { } shouldPatchExecuteScript(script) { - if (!script || typeof script !== 'string') { + if (!script) { return true; } + // A non-string script (a function passed to .execute()/.executeAsyncScript()) + // is always a user script — the plugin's own scan scripts are always strings + // carrying the browserstack_executor token. Scan it. + if (typeof script !== 'string') { + return false; + } + return ( script.toLowerCase().indexOf('browserstack_executor') !== -1 || script.toLowerCase().indexOf('browserstack_accessibility_automation_script') !== -1 From 9659b4544f82095e2e23eefc4bd7f16e5b25ed97 Mon Sep 17 00:00:00 2001 From: Kamalpreet Kaur Date: Wed, 10 Jun 2026 18:58:29 +0530 Subject: [PATCH 3/3] test(accessibility): cover commandWrapper prototype patching + executeScript guard Adds unit tests for the two accessibility command-wrapping fixes: - shouldPatchExecuteScript: empty -> skip, internal browserstack_executor / accessibility scripts -> skip, user string scripts -> scan, and (regression) function-form user scripts -> scan. - commandWrapper: wraps the own `command` of object-export (web-element) commands and the prototype `command` of class-export commands (executeScript) without creating a phantom static; verifies the wrapped command triggers performScan, delegates to the original, honours the recursion guard, and scans function-form scripts. Drives commandWrapper deterministically by stubbing require.resolve('nightwatch') and injecting fake command modules via require.cache. Co-Authored-By: Claude Opus 4.8 (1M context) --- test/src/accessibilityAutomation.js | 136 ++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 test/src/accessibilityAutomation.js diff --git a/test/src/accessibilityAutomation.js b/test/src/accessibilityAutomation.js new file mode 100644 index 0000000..44aa6bf --- /dev/null +++ b/test/src/accessibilityAutomation.js @@ -0,0 +1,136 @@ +const path = require('path'); +const Module = require('module'); +const sinon = require('sinon'); +const {expect} = require('chai'); + +// accessibilityAutomation participates in a circular require +// (accessibilityAutomation -> helper -> logPatcher -> testObservability -> accessibilityAutomation), +// and testObservability instantiates AccessibilityAutomation at load time. Warm up the chain via +// helper first so the class is fully exported before we require it directly. +require('../../src/utils/helper'); +const AccessibilityAutomation = require('../../src/accessibilityAutomation'); +const AccessibilityScripts = require('../../src/scripts/accessibilityScripts'); + +describe('AccessibilityAutomation.shouldPatchExecuteScript', () => { + let instance; + + before(() => { + instance = new AccessibilityAutomation(); + }); + + it('returns true (skip scan) for an empty/undefined script', () => { + expect(instance.shouldPatchExecuteScript(undefined)).to.eq(true); + expect(instance.shouldPatchExecuteScript(null)).to.eq(true); + expect(instance.shouldPatchExecuteScript('')).to.eq(true); + }); + + it('returns true (skip scan) for the plugin\'s internal browserstack scripts', () => { + expect(instance.shouldPatchExecuteScript('browserstack_executor: {"action":"appAllyScan"}')).to.eq(true); + expect(instance.shouldPatchExecuteScript('BROWSERSTACK_EXECUTOR: {}')).to.eq(true); + expect(instance.shouldPatchExecuteScript('browserstack_accessibility_automation_script')).to.eq(true); + }); + + it('returns false (trigger scan) for a normal user string script', () => { + expect(instance.shouldPatchExecuteScript('mobile:scroll')).to.eq(false); + expect(instance.shouldPatchExecuteScript('return document.title')).to.eq(false); + }); + + it('returns false (trigger scan) for a function-form user script', () => { + // Regression: a function script was previously treated as "skip", so + // browser.execute(function(){...}) never triggered an accessibility scan. + expect(instance.shouldPatchExecuteScript(function () {})).to.eq(false); + expect(instance.shouldPatchExecuteScript(() => {})).to.eq(false); + }); +}); + +describe('AccessibilityAutomation.commandWrapper', () => { + const fakeMain = path.join('/tmp', 'fake-nightwatch', 'index.js'); + const fakeDir = path.dirname(fakeMain); + const objPath = path.join(fakeDir, 'api/web-element/commands', 'objCmd.js'); + const classPath = path.join(fakeDir, 'api/client-commands/document', 'executeScript.js'); + + let resolveStub; let instance; let perfStub; let objModule; let ClassModule; let origObjCmd; let origClassCmd; + + beforeEach(async () => { + const origResolve = Module._resolveFilename; + resolveStub = sinon.stub(Module, '_resolveFilename').callsFake(function (request, ...rest) { + if (request === 'nightwatch') { + return fakeMain; + } + if (request === objPath || request === classPath) { + return request; + } + + return origResolve.call(this, request, ...rest); + }); + + // Object-export command (web-element style): `module.exports.command = fn`. + origObjCmd = function () { return 'obj-orig' }; + objModule = {command: origObjCmd}; + + // Class-export command (executeScript style): `command` lives on the prototype. + origClassCmd = function () { return 'class-orig' }; + ClassModule = class ExecuteScript {}; + ClassModule.prototype.command = origClassCmd; + + require.cache[objPath] = {id: objPath, filename: objPath, loaded: true, exports: objModule}; + require.cache[classPath] = {id: classPath, filename: classPath, loaded: true, exports: ClassModule}; + + AccessibilityScripts.commandsToWrap = [ + {method: 'command', path: 'api/web-element/commands', name: ['objCmd']}, + {method: 'command', path: 'api/client-commands/document', name: ['executeScript']} + ]; + + global.browser = {}; + instance = new AccessibilityAutomation(); + perfStub = sinon.stub(instance, 'performScan').resolves('scanned'); + + await instance.commandWrapper(); + }); + + afterEach(() => { + resolveStub.restore(); + delete require.cache[objPath]; + delete require.cache[classPath]; + delete global.browser; + AccessibilityScripts.commandsToWrap = null; + }); + + it('wraps the own `command` of an object-export (web-element) command', () => { + expect(objModule.command).to.be.a('function'); + expect(objModule.command).to.not.eq(origObjCmd); + }); + + it('wraps the prototype `command` of a class-export command, not a static', () => { + expect(ClassModule.prototype.command).to.not.eq(origClassCmd); + // The original bug patched a non-existent static `command`; ensure we did NOT create one. + expect(Object.prototype.hasOwnProperty.call(ClassModule, 'command')).to.eq(false); + }); + + it('class-export wrapped command triggers performScan and delegates to the original', async () => { + const result = await new ClassModule().command('mobile:scroll'); + + expect(perfStub.calledOnce).to.eq(true); + expect(perfStub.firstCall.args[1]).to.eq('executeScript'); + expect(result).to.eq('class-orig'); + }); + + it('skips performScan for the plugin\'s internal browserstack_executor script (recursion guard)', async () => { + await new ClassModule().command('browserstack_executor: {"action":"appAllyScan"}'); + + expect(perfStub.called).to.eq(false); + }); + + it('triggers performScan for a function-form user script passed to execute', async () => { + await new ClassModule().command(function () {}); + + expect(perfStub.calledOnce).to.eq(true); + }); + + it('triggers performScan for a non-execute object-export command', async () => { + await objModule.command(); + + expect(perfStub.calledOnce).to.eq(true); + expect(perfStub.firstCall.args[1]).to.eq('objCmd'); + }); +});