fix(ocr-service): drop node-tesseract-ocr and call the tesseract binary with execFile

.changeset/ocr-service-drop-node-tesseract-ocr.md

@@ -0,0 +1,7 @@
+---
+'@wdio/ocr-service': patch
+---
+
+Remove the `node-tesseract-ocr` dependency from `@wdio/ocr-service`. It carried an unpatched critical advisory (GHSA-8j44-735h-w4w2, OS command injection in `recognize()`) and has had no release since 2022, so every project that installed the OCR service got one `npm audit` finding it could not clear.
+
+The service now runs the system `tesseract` binary itself through `child_process.execFile` with an argument array, so the screenshot path and the options never pass through a shell. The command that is run is the same as before (`tesseract <file> stdout -l <lang> --oem 1 --psm 3 alto`), so the OCR output does not change. `isSystemTesseractAvailable()` uses `execFileSync` for the same reason.

packages/ocr-service/package.json

@@ -37,7 +37,6 @@
     "@wdio/types": "^10.0.1",
     "fuse.js": "^7.5.0",
     "jimp": "^1.6.1",
-    "node-tesseract-ocr": "^2.2.1",
     "tesseract.js": "^7.0.0",
     "xml2js": "^0.6.2"
   },

packages/ocr-service/src/utils/tesseract.ts

@@ -1,22 +1,52 @@
-import { execSync } from 'node:child_process'
+import { execFile, execFileSync } from 'node:child_process'
+import { promisify } from 'node:util'
 import { createWorker, PSM } from 'tesseract.js'
-import { recognize } from 'node-tesseract-ocr'
 import { parseString } from 'xml2js'
 import type { GetOcrData, Line, ParseWordData, Rectangles, TessaractDataOptions, UnprocessedNodejsBlock, UnprocessedSystemBlock, Words } from '../types.js'
 
+const execFileAsync = promisify(execFile)
+// Tesseract writes the ALTO XML of a full-screen capture to stdout. 1 MB (the Node
+// default) is too small for a busy screen, so give it room.
+const TESSERACT_MAX_STDOUT_BYTES = 10 * 1024 * 1024
+
 export function isSystemTesseractAvailable(tesseractName: string = ''): boolean {
     const binary = tesseractName || 'tesseract'
-    const command = [binary, '--version'].join(' ')
 
     try {
-        execSync(command)
+        // execFileSync with an argument array never starts a shell, so the binary
+        // name is passed through as-is.
+        execFileSync(binary, ['--version'], { stdio: 'ignore' })
     } catch (_ign) {
         return false
     }
 
     return true
 }
 
+type SystemTesseractOptions = {
+    lang: string;
+    oem: number;
+    psm: number;
+    presets: string[];
+}
+
+/**
+ * Run the system Tesseract binary and return what it writes to stdout.
+ *
+ * The file path and every option go to the binary as an argument array, so no
+ * shell ever sees them. This replaces `node-tesseract-ocr`, which built one shell
+ * command string from the file path (GHSA-8j44-735h-w4w2) and has no patched
+ * release.
+ */
+export async function runSystemTesseract(filePath: string, { lang, oem, psm, presets }: SystemTesseractOptions): Promise<string> {
+    // `-l` and `--psm` must come before any config file name such as `alto`, see
+    // https://github.com/tesseract-ocr/tesseract/blob/master/doc/tesseract.1.asc
+    const args = [filePath, 'stdout', '-l', lang, '--oem', String(oem), '--psm', String(psm), ...presets]
+    const { stdout } = await execFileAsync('tesseract', args, { maxBuffer: TESSERACT_MAX_STDOUT_BYTES })
+
+    return stdout
+}
+
 export function parseWordDataFromText(attributes: string[]): ParseWordData {
     let bbox = {
         left: 0,
@@ -164,10 +194,9 @@ export async function getSystemOcrData(options: TessaractDataOptions): Promise<G
         const jsonSingleWords: Words[] = []
         const jsonWordStrings: Line[] = []
         let composedBlocks: UnprocessedSystemBlock[] = []
-        const result = await recognize(filePath, {
+        const result = await runSystemTesseract(filePath, {
             lang: language,
             oem: 1,
-            // https://github.com/tesseract-ocr/tesseract/blob/master/doc/tesseract.1.asc
             psm: 3,
             presets: ['alto'],
         })

packages/ocr-service/tests/commands/tesseract.test.ts

@@ -1,12 +1,12 @@
 import { describe, it, expect, beforeEach, vi } from 'vitest'
-import { execSync } from 'node:child_process'
+import { execFile, execFileSync } from 'node:child_process'
 import { createWorker } from 'tesseract.js'
-import { recognize } from 'node-tesseract-ocr'
 import { parseString } from 'xml2js'
-import { getNodeOcrData, getSystemOcrData, isSystemTesseractAvailable, parseWordDataFromText } from '../../src/utils/tesseract.js'
+import { getNodeOcrData, getSystemOcrData, isSystemTesseractAvailable, parseWordDataFromText, runSystemTesseract } from '../../src/utils/tesseract.js'
 
 vi.mock('child_process', () => ({
-    execSync: vi.fn()
+    execFile: vi.fn(),
+    execFileSync: vi.fn()
 }))
 vi.mock('tesseract.js', () => ({
     createWorker: vi.fn(() => ({
@@ -21,9 +21,6 @@ vi.mock('tesseract.js', () => ({
         AUTO: 'auto'
     }
 }))
-vi.mock('node-tesseract-ocr', () => ({
-    recognize: vi.fn()
-}))
 vi.mock('xml2js', () => ({
     parseString: vi.fn()
 }))
@@ -34,32 +31,87 @@ describe('isSystemTesseractAvailable', () => {
     })
 
     it('returns true when Tesseract is available', () => {
-        vi.mocked(execSync).mockImplementation(() => '')
+        vi.mocked(execFileSync).mockImplementation(() => '')
 
         expect(isSystemTesseractAvailable()).toBe(true)
-        expect(execSync).toHaveBeenCalledWith('tesseract --version')
+        expect(execFileSync).toHaveBeenCalledWith('tesseract', ['--version'], { stdio: 'ignore' })
     })
 
     it('returns false when Tesseract is not available', () => {
-        vi.mocked(execSync).mockImplementation(() => {
+        vi.mocked(execFileSync).mockImplementation(() => {
             throw new Error('command not found')
         })
 
         expect(isSystemTesseractAvailable()).toBe(false)
-        expect(execSync).toHaveBeenCalledWith('tesseract --version')
+        expect(execFileSync).toHaveBeenCalledWith('tesseract', ['--version'], { stdio: 'ignore' })
     })
 
     it('checks the specified Tesseract binary name', () => {
         const customBinary = 'custom-tesseract'
         // @ts-ignore
-        vi.mocked(execSync).mockImplementation((command) => {
-            if (!command.includes(customBinary)) {
+        vi.mocked(execFileSync).mockImplementation((file) => {
+            if (file !== customBinary) {
                 throw new Error('command not found')
             }
         })
 
         expect(isSystemTesseractAvailable(customBinary)).toBe(true)
-        expect(execSync).toHaveBeenCalledWith(`${customBinary} --version`)
+        expect(execFileSync).toHaveBeenCalledWith(customBinary, ['--version'], { stdio: 'ignore' })
+    })
+
+    it('never starts a shell, so a binary name with shell characters is passed through literally', () => {
+        const hostileBinary = 'tesseract; touch /tmp/pwned'
+        vi.mocked(execFileSync).mockImplementation(() => '')
+
+        isSystemTesseractAvailable(hostileBinary)
+
+        expect(execFileSync).toHaveBeenCalledWith(hostileBinary, ['--version'], { stdio: 'ignore' })
+    })
+})
+
+describe('runSystemTesseract', () => {
+    beforeEach(() => {
+        vi.clearAllMocks()
+    })
+
+    it('runs the tesseract binary with an argument array and returns stdout', async () => {
+        // @ts-ignore
+        vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => {
+            callback(null, { stdout: '<alto/>', stderr: '' })
+        })
+
+        const result = await runSystemTesseract('/fake/path/image.png', { lang: 'eng', oem: 1, psm: 3, presets: ['alto'] })
+
+        expect(result).toBe('<alto/>')
+        expect(execFile).toHaveBeenCalledWith(
+            'tesseract',
+            ['/fake/path/image.png', 'stdout', '-l', 'eng', '--oem', '1', '--psm', '3', 'alto'],
+            { maxBuffer: 10 * 1024 * 1024 },
+            expect.any(Function),
+        )
+    })
+
+    it('passes a file path with shell characters through as one literal argument', async () => {
+        const hostilePath = '/tmp/image.png"; touch /tmp/pwned; echo "'
+        // @ts-ignore
+        vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => {
+            callback(null, { stdout: '', stderr: '' })
+        })
+
+        await runSystemTesseract(hostilePath, { lang: 'eng', oem: 1, psm: 3, presets: ['alto'] })
+
+        const [, args] = vi.mocked(execFile).mock.calls[0]
+        expect(args[0]).toBe(hostilePath)
+    })
+
+    it('rejects when the binary fails', async () => {
+        // @ts-ignore
+        vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => {
+            callback(new Error('Recognition failed'), { stdout: '', stderr: '' })
+        })
+
+        await expect(runSystemTesseract('/fake/path/image.png', { lang: 'eng', oem: 1, psm: 3, presets: ['alto'] }))
+            .rejects.toThrow('Recognition failed')
     })
 })
 
@@ -334,6 +386,13 @@ describe('getSystemOcrData', () => {
     const mockLanguage = 'eng'
     const options = { filePath: mockFilePath, language: mockLanguage }
 
+    function mockTesseractStdout(stdout: string) {
+        // @ts-ignore
+        vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => {
+            callback(null, { stdout, stderr: '' })
+        })
+    }
+
     beforeEach(() => {
         vi.clearAllMocks()
         mockedParsedTextSting = '\n\t\t\nNext-gen browser and mobile automation\n\ntest framework for Node.js\n\t'
@@ -376,7 +435,7 @@ describe('getSystemOcrData', () => {
     it('successfully processes OCR data', async () => {
         const mockResult = '<alto><Layout><Page><PrintSpace><ComposedBlock><TextBlock><TextLine HPOS="100" VPOS="200" WIDTH="300" HEIGHT="400"><String CONTENT="\nNext-gen browser and mobile automation\n\ntest framework for Node.js" HPOS="100" VPOS="200" WIDTH="50" HEIGHT="20" WC="0.95"/></TextLine></TextBlock></ComposedBlock></PrintSpace></Page></Layout></alto>'
 
-        vi.mocked(recognize).mockResolvedValue(mockResult)
+        mockTesseractStdout(mockResult)
         vi.mocked(parseString).mockImplementation((_input, callback) => {
             // @ts-ignore
             callback(null, {
@@ -391,19 +450,22 @@ describe('getSystemOcrData', () => {
 
         const result = await getSystemOcrData(options)
 
-        expect(recognize).toHaveBeenCalledWith(mockFilePath, {
-            lang: mockLanguage,
-            oem: 1,
-            psm: 3,
-            presets: ['alto']
-        })
+        expect(execFile).toHaveBeenCalledWith(
+            'tesseract',
+            [mockFilePath, 'stdout', '-l', mockLanguage, '--oem', '1', '--psm', '3', 'alto'],
+            expect.any(Object),
+            expect.any(Function),
+        )
         expect(result.text).toMatchSnapshot()
         expect(result.lines.length).toEqual(2)
         expect(result.words.length).toEqual(4)
     })
 
     it('handles recognition errors', async () => {
-        vi.mocked(recognize).mockRejectedValue(new Error('Recognition failed'))
+        // @ts-ignore
+        vi.mocked(execFile).mockImplementation((_file, _args, _options, callback) => {
+            callback(new Error('Recognition failed'), { stdout: '', stderr: '' })
+        })
 
         await expect(getSystemOcrData({ filePath: mockFilePath, language: mockLanguage }))
             .rejects
@@ -413,7 +475,7 @@ describe('getSystemOcrData', () => {
     it('handles XML parsing errors', async () => {
         const mockResult = 'Invalid XML content'
 
-        vi.mocked(recognize).mockResolvedValue(mockResult)
+        mockTesseractStdout(mockResult)
         vi.mocked(parseString).mockImplementation((_input, callback) => {
             // @ts-ignore
             callback(new Error('Parsing failed'), null)
@@ -427,7 +489,7 @@ describe('getSystemOcrData', () => {
     it('handles cases with no text found', async () => {
         const mockResult = '<alto><Layout><Page><PrintSpace><ComposedBlock/></PrintSpace></Page></Layout></alto>'
 
-        vi.mocked(recognize).mockResolvedValue(mockResult)
+        mockTesseractStdout(mockResult)
         vi.mocked(parseString).mockImplementation((_input, callback) => {
             // @ts-ignore
             callback(null, { alto: { Layout: [{ Page: [{ PrintSpace: [{ ComposedBlock: [] }] }] }] } })
@@ -444,7 +506,7 @@ describe('getSystemOcrData', () => {
 
         beforeEach(() => {
             vi.clearAllMocks()
-            vi.mocked(recognize).mockResolvedValue('<alto>...</alto>') // Base mock for OCR recognize function
+            mockTesseractStdout('<alto>...</alto>') // Base mock for the tesseract binary
         })
 
         function setupStringMock(missingKey) {