Skip to content

Commit

Permalink
fix: add column, line, and method check to integrity check (#25094)
Browse files Browse the repository at this point in the history
  • Loading branch information
ryanthemanuel authored Dec 12, 2022
1 parent 2062670 commit 8888cd9
Show file tree
Hide file tree
Showing 8 changed files with 195 additions and 13 deletions.
2 changes: 1 addition & 1 deletion .circleci/cache-version.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# Bump this version to force CI to re-create the cache from scratch.

12-05-22
12-12-22
7 changes: 5 additions & 2 deletions .circleci/workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ mainBuildFilters: &mainBuildFilters
only:
- develop
- /^release\/\d+\.\d+\.\d+$/
- 'ryanm/fix/column-line'

# usually we don't build Mac app - it takes a long time
# but sometimes we want to really confirm we are doing the right thing
Expand All @@ -36,13 +37,15 @@ macWorkflowFilters: &darwin-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/column-line', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
linuxArm64WorkflowFilters: &linux-arm64-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/column-line', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
Expand All @@ -60,7 +63,7 @@ windowsWorkflowFilters: &windows-workflow-filters
when:
or:
- equal: [ develop, << pipeline.git.branch >> ]
- equal: [ 'mschile/fix_windows_flake', << pipeline.git.branch >> ]
- equal: [ 'ryanm/fix/column-line', << pipeline.git.branch >> ]
- matches:
pattern: /^release\/\d+\.\d+\.\d+$/
value: << pipeline.git.branch >>
Expand Down Expand Up @@ -126,7 +129,7 @@ commands:
- run:
name: Check current branch to persist artifacts
command: |
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "ryanm/fix/issue-with-integrity-check" ]]; then
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "ryanm/fix/column-line" ]]; then
echo "Not uploading artifacts or posting install comment for this branch."
circleci-agent step halt
fi
Expand Down
124 changes: 124 additions & 0 deletions patches/bytenode+1.3.7.dev.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
diff --git a/node_modules/bytenode/lib/cli.js b/node_modules/bytenode/lib/cli.js
index 9f57493..3ba173e 100755
--- a/node_modules/bytenode/lib/cli.js
+++ b/node_modules/bytenode/lib/cli.js
@@ -2,11 +2,15 @@

'use strict';

-const fs = require('fs');
-const path = require('path');
-const wrap = require('module').wrap;
-const spawnSync = require('child_process').spawnSync;
-const bytenode = require('./index.js');
+import fs from 'fs';
+import path from 'path';
+import * as mod from 'module';
+import { spawnSync } from 'child_process';
+import * as bytenode from './index.js';
+import * as url from 'url';
+
+const __filename = url.fileURLToPath(import.meta.url);
+const __dirname = url.fileURLToPath(new URL('.', import.meta.url));

const args = process.argv.slice(2);

@@ -100,7 +104,7 @@ if (program.flags.includes('--compile')) {
if (program.flags.includes('--no-module')) {
process.stdout.write(bytenode.compileCode(script));
} else {
- process.stdout.write(bytenode.compileCode(wrap(script)));
+ process.stdout.write(bytenode.compileCode(mod.wrap(script)));
}
} catch (error) {
console.error(error);
@@ -136,13 +140,6 @@ if (program.flags.includes('--compile')) {
$ echo 'console.log("Hello");' | bytenode --compile - > hello.jsc
compile from stdin and save to \`hello.jsc\`
`);
-} else if (program.flags.includes('--version') && program.flags.length === 1 && program.files.length === 0) {
- const pkg = require('../package.json');
- console.log(pkg.name, pkg.version);
- console.log('Node', process.versions.node);
- if (process.versions.electron) {
- console.log('Electron', process.versions.electron);
- }
} else {
try {
spawnSync(program.nodeBin, [
diff --git a/node_modules/bytenode/lib/index.js b/node_modules/bytenode/lib/index.js
index cdd98cc..635bc27 100644
--- a/node_modules/bytenode/lib/index.js
+++ b/node_modules/bytenode/lib/index.js
@@ -1,11 +1,13 @@
'use strict';

-const fs = require('fs');
-const vm = require('vm');
-const v8 = require('v8');
-const path = require('path');
-const Module = require('module');
-const fork = require('child_process').fork;
+import fs from 'fs';
+import vm from 'vm';
+import v8 from 'v8';
+import path from 'path';
+import Module from 'module';
+import { fork } from 'child_process';
+import { createRequire } from 'module';
+import * as url from 'url';

v8.setFlagsFromString('--no-lazy');

@@ -46,10 +48,12 @@ const compileElectronCode = function (javascriptCode) {
return new Promise((resolve, reject) => {
let data = Buffer.from([]);

+ const require = createRequire(import.meta.url)
const electronPath = path.join(path.dirname(require.resolve('electron')), 'cli.js');
if (!fs.existsSync(electronPath)) {
throw new Error('Electron not installed');
}
+ const __dirname = url.fileURLToPath(new URL('.', import.meta.url));
const bytenodePath = path.join(__dirname, 'cli.js');

// create a subprocess in which we run Electron as our Node and V8 engine
@@ -249,7 +253,7 @@ const runBytecodeFile = function (filename) {
return runBytecode(bytecodeBuffer);
};

-Module._extensions[COMPILED_EXTNAME] = function (fileModule, filename) {
+const runBytecodeAsModule = function (fileModule, filename) {
const bytecodeBuffer = fs.readFileSync(filename);

fixBytecode(bytecodeBuffer);
@@ -333,14 +337,13 @@ const loaderCode = function (targetPath) {
`;
};

-global.bytenode = {
+export {
compileCode,
compileFile,
compileElectronCode,
runBytecode,
runBytecodeFile,
addLoaderFile,
- loaderCode
+ loaderCode,
+ runBytecodeAsModule,
};
-
-module.exports = global.bytenode;
diff --git a/node_modules/bytenode/package.json b/node_modules/bytenode/package.json
index 6caff7c..72e2c46 100644
--- a/node_modules/bytenode/package.json
+++ b/node_modules/bytenode/package.json
@@ -3,6 +3,7 @@
"version": "1.3.7",
"description": "A minimalist bytecode compiler for Node.js",
"main": "lib/index.js",
+ "type": "module",
"bin": "lib/cli.js",
"types": "lib/index.d.ts",
"files": [
3 changes: 2 additions & 1 deletion scripts/binary/binary-cleanup.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const esbuild = require('esbuild')
const snapshotMetadata = require('@tooling/v8-snapshot/cache/prod-darwin/snapshot-meta.cache.json')
const tempDir = require('temp-dir')
const workingDir = path.join(tempDir, 'binary-cleanup-workdir')
const bytenode = require('bytenode')

fs.ensureDirSync(workingDir)

Expand Down Expand Up @@ -137,6 +136,8 @@ const createServerEntryPointBundle = async (buildAppDir) => {
console.log(`compiling server entry point bundle to ${path.join(buildAppDir, 'packages', 'server', 'index.jsc')}`)

// Use bytenode to compile the entry point bundle. This will save time on the v8 compile step and ensure the integrity of the entry point
const bytenode = await import('bytenode')

await bytenode.compileFile({
filename: path.join(buildAppDir, 'packages', 'server', 'index.js'),
output: path.join(buildAppDir, 'packages', 'server', 'index.jsc'),
Expand Down
8 changes: 4 additions & 4 deletions scripts/binary/binary-entry-point-source.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
const Module = require('module')
const path = require('path')
import Module from 'module'
import path from 'path'
import { runBytecodeAsModule } from 'bytenode'

process.env.CYPRESS_INTERNAL_ENV = process.env.CYPRESS_INTERNAL_ENV || 'production'
try {
require('bytenode')
const filename = path.join(__dirname, 'packages', 'server', 'index.jsc')
const dirname = path.dirname(filename)

Module._extensions['.jsc']({
runBytecodeAsModule({
require: module.require,
id: filename,
filename,
Expand Down
44 changes: 39 additions & 5 deletions scripts/binary/binary-integrity-check-source.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ const OrigError = Error
const captureStackTrace = Error.captureStackTrace
const toString = Function.prototype.toString
const callFn = Function.call
const filter = Array.prototype.filter
const startsWith = String.prototype.startsWith

const integrityErrorMessage = `
We detected an issue with the integrity of the Cypress binary. It may have been modified and cannot run. We recommend re-installing the Cypress binary with:
Expand All @@ -22,7 +24,7 @@ const stackIntegrityCheck = function stackIntegrityCheck (options) {
const tempError = new OrigError

captureStackTrace(tempError, arguments.callee)
const stack = tempError.stack.filter((frame) => !frame.getFileName().startsWith('node:internal') && !frame.getFileName().startsWith('node:electron'))
const stack = filter.call(tempError.stack, (frame) => !startsWith.call(frame.getFileName(), 'node:internal') && !startsWith.call(frame.getFileName(), 'node:electron'))

OrigError.prepareStackTrace = originalPrepareStackTrace
OrigError.stackTraceLimit = originalStackTraceLimit
Expand All @@ -33,9 +35,11 @@ const stackIntegrityCheck = function stackIntegrityCheck (options) {
}

for (let index = 0; index < options.stackToMatch.length; index++) {
const { functionName: expectedFunctionName, fileName: expectedFileName } = options.stackToMatch[index]
const { functionName: expectedFunctionName, fileName: expectedFileName, line: expectedLineNumber, column: expectedColumnNumber } = options.stackToMatch[index]
const actualFunctionName = stack[index].getFunctionName()
const actualFileName = stack[index].getFileName()
const actualColumnNumber = stack[index].getColumnNumber()
const actualLineNumber = stack[index].getLineNumber()

if (expectedFunctionName && actualFunctionName !== expectedFunctionName) {
console.error(`Integrity check failed with expected function name ${expectedFunctionName} but got ${actualFunctionName}`)
Expand All @@ -46,21 +50,45 @@ const stackIntegrityCheck = function stackIntegrityCheck (options) {
console.error(`Integrity check failed with expected file name ${expectedFileName} but got ${actualFileName}`)
throw new Error(integrityErrorMessage)
}

if (expectedLineNumber && actualLineNumber !== expectedLineNumber) {
console.error(`Integrity check failed with expected line number ${expectedLineNumber} but got ${actualLineNumber}`)
throw new Error(integrityErrorMessage)
}

if (expectedColumnNumber && actualColumnNumber !== expectedColumnNumber) {
console.error(`Integrity check failed with expected column number ${expectedColumnNumber} but got ${actualColumnNumber}`)
throw new Error(integrityErrorMessage)
}
}
}

function validateStartsWith () {
if (startsWith.call !== callFn) {
console.error(`Integrity check failed for startsWith.call`)
throw new Error(integrityErrorMessage)
}
}

function validateFilter () {
if (filter.call !== callFn) {
console.error(`Integrity check failed for filter.call`)
throw new Error(integrityErrorMessage)
}
}

function validateToString () {
if (toString.call !== callFn) {
console.error(`Integrity check failed for toString.call`)
throw new Error('Integrity check failed for toString.call')
throw new Error(integrityErrorMessage)
}
}

function validateElectron (electron) {
// Hard coded function as this is electron code and there's not an easy way to get the function string at package time. If this fails on an updated version of electron, we'll need to update this.
if (toString.call(electron.app.getAppPath) !== 'function getAppPath() { [native code] }') {
console.error(`Integrity check failed for toString.call(electron.app.getAppPath)`)
throw new Error(`Integrity check failed for toString.call(electron.app.getAppPath)`)
throw new Error(integrityErrorMessage)
}
}

Expand Down Expand Up @@ -106,6 +134,8 @@ function integrityCheck (options) {
const crypto = require('crypto')

// 1. Validate that the native functions we are using haven't been tampered with
validateStartsWith()
validateFilter()
validateToString()
validateElectron(electron)
validateFs(fs)
Expand Down Expand Up @@ -143,13 +173,17 @@ function integrityCheck (options) {
fileName: 'evalmachine.<anonymous>',
},
{
functionName: 'Module2._extensions.<computed>',
functionName: 'v',
// eslint-disable-next-line no-undef
fileName: [appPath, 'index.js'].join(PATH_SEP),
line: 1,
column: 2573,
},
{
// eslint-disable-next-line no-undef
fileName: [appPath, 'index.js'].join(PATH_SEP),
line: 1,
column: 2764,
},
],
})
Expand Down
2 changes: 2 additions & 0 deletions scripts/binary/binary-sources.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const getBinaryEntryPointSource = async () => {
bundle: true,
platform: 'node',
write: false,
minify: true,
treeShaking: true,
})

return esbuildResult.outputFiles[0].text
Expand Down
18 changes: 18 additions & 0 deletions scripts/binary/smoke.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,24 @@ const runIntegrityTest = async function (buildAppExecutable, buildAppDir, e2e) {
const allowList = ['regeneratorRuntime', '__core-js_shared__', 'getSnapshotResult', 'supportTypeScript']

await testAlteringEntryPoint(`(${compareGlobals.toString()})()`, `extra keys in electron process: ${allowList}\nIntegrity check failed with expected stack length 9 but got 10`)

const testTemporarilyRewritingEntryPoint = async () => {
const file = path.join(buildAppDir, 'index.js')
const backupFile = path.join(buildAppDir, 'index.js.bak')
const contents = await fs.readFile(file)

// Backup state
await fs.move(file, backupFile)

// Modify app
await fs.writeFile(file, `console.log("rewritten code");const fs=require('fs');const { join } = require('path');fs.writeFileSync(join(__dirname,'index.js'),fs.readFileSync(join(__dirname,'index.js.bak')));${contents}`)
await runErroringProjectTest(buildAppExecutable, e2e, 'temporarily rewriting index.js', 'Integrity check failed with expected column number 2573 but got')

// Restore original state
await fs.move(backupFile, file, { overwrite: true })
}

await testTemporarilyRewritingEntryPoint()
}

const test = async function (buildAppExecutable, buildAppDir) {
Expand Down

5 comments on commit 8888cd9

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 8888cd9 Dec 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux arm64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.1.0/linux-arm64/develop-8888cd9e211e608f9bbac81478667b2877dab76e/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 8888cd9 Dec 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.1.0/linux-x64/develop-8888cd9e211e608f9bbac81478667b2877dab76e/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 8888cd9 Dec 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.1.0/darwin-x64/develop-8888cd9e211e608f9bbac81478667b2877dab76e/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 8888cd9 Dec 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.1.0/win32-x64/develop-8888cd9e211e608f9bbac81478667b2877dab76e/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 8888cd9 Dec 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin arm64 version of the Test Runner.

Learn more about this pre-release platform-specific build at https://on.cypress.io/installing-cypress#Install-pre-release-version.

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.1.0/darwin-arm64/develop-8888cd9e211e608f9bbac81478667b2877dab76e/cypress.tgz

Please sign in to comment.