Skip to content

Commit

Permalink
feat: supply system log to trusted plugins via FD (#5391)
Browse files Browse the repository at this point in the history
* feat: supply system log to trusted plugins via FD

* chore: add comment about extracting

* chore: update test name

* fix: tests

* refactor: extract isTrustedPlugin

* refactor: provide via "systemLog" function

* fix: add systemLog to types
  • Loading branch information
Skn0tt authored Nov 20, 2023
1 parent e5e567e commit 8fee92b
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 10 deletions.
5 changes: 5 additions & 0 deletions packages/build/src/core/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ const tExecBuild = async function ({
logs,
debug,
systemLog,
systemLogFile,
verbose,
timers: timersA,
sendStatus,
Expand Down Expand Up @@ -264,6 +265,7 @@ export const runAndReportBuild = async function ({
logs,
debug,
systemLog,
systemLogFile,
verbose,
timers,
sendStatus,
Expand Down Expand Up @@ -315,6 +317,7 @@ export const runAndReportBuild = async function ({
logs,
debug,
systemLog,
systemLogFile,
verbose,
timers,
sendStatus,
Expand Down Expand Up @@ -418,6 +421,7 @@ const initAndRunBuild = async function ({
logs,
debug,
systemLog,
systemLogFile,
verbose,
sendStatus,
saveConfig,
Expand Down Expand Up @@ -466,6 +470,7 @@ const initAndRunBuild = async function ({
timers: timersA,
featureFlags,
quiet,
systemLogFile,
})

try {
Expand Down
13 changes: 12 additions & 1 deletion packages/build/src/plugins/child/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { getNewEnvChanges, setEnvChanges } from '../../env/changes.js'
import { logPluginMethodStart, logPluginMethodEnd } from '../../log/messages/ipc.js'

import { cloneNetlifyConfig, getConfigMutations } from './diff.js'
import { getSystemLog } from './systemLog.js'
import { getUtils } from './utils.js'

// Run a specific plugin event handler
Expand All @@ -11,9 +12,19 @@ export const run = async function (
) {
const method = methods[event]
const runState = {}
const systemLog = getSystemLog()
const utils = getUtils({ event, constants, runState })
const netlifyConfigCopy = cloneNetlifyConfig(netlifyConfig)
const runOptions = { utils, constants, inputs, netlifyConfig: netlifyConfigCopy, packageJson, error, featureFlags }
const runOptions = {
utils,
constants,
inputs,
netlifyConfig: netlifyConfigCopy,
packageJson,
error,
featureFlags,
systemLog,
}

const envBefore = setEnvChanges(envChanges)

Expand Down
15 changes: 15 additions & 0 deletions packages/build/src/plugins/child/systemLog.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { appendFileSync, openSync } from 'fs'

const systemLogLocation = '/dev/fd/4'

export const getSystemLog = () => {
try {
// throws if system log wasn't hooked up
const fd = openSync(systemLogLocation, 'a')
return (message) => {
appendFileSync(fd, `${message}\n`)
}
} catch {
return
}
}
11 changes: 8 additions & 3 deletions packages/build/src/plugins/spawn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
logIncompatiblePlugins,
logLoadingIntegration,
} from '../log/messages/compatibility.js'
import { isTrustedPlugin } from '../steps/plugin.js'
import { measureDuration } from '../time/main.js'

import { getEventFromChild } from './ipc.js'
Expand All @@ -23,7 +24,7 @@ const CHILD_MAIN_FILE = fileURLToPath(new URL('child/main.js', import.meta.url))
// (for both security and safety reasons)
// - logs can be buffered which allows manipulating them for log shipping,
// transforming and parallel plugins
const tStartPlugins = async function ({ pluginsOptions, buildDir, childEnv, logs, debug, quiet }) {
const tStartPlugins = async function ({ pluginsOptions, buildDir, childEnv, logs, debug, quiet, systemLogFile }) {
if (!quiet) {
logRuntime(logs, pluginsOptions)
logLoadingPlugins(logs, pluginsOptions, debug)
Expand All @@ -34,14 +35,16 @@ const tStartPlugins = async function ({ pluginsOptions, buildDir, childEnv, logs
logIncompatiblePlugins(logs, pluginsOptions)

const childProcesses = await Promise.all(
pluginsOptions.map(({ pluginDir, nodePath }) => startPlugin({ pluginDir, nodePath, buildDir, childEnv })),
pluginsOptions.map(({ pluginDir, nodePath, pluginPackageJson }) =>
startPlugin({ pluginDir, nodePath, buildDir, childEnv, systemLogFile, pluginPackageJson }),
),
)
return { childProcesses }
}

export const startPlugins = measureDuration(tStartPlugins, 'start_plugins')

const startPlugin = async function ({ pluginDir, nodePath, buildDir, childEnv }) {
const startPlugin = async function ({ pluginDir, nodePath, buildDir, childEnv, systemLogFile, pluginPackageJson }) {
const childProcess = execaNode(CHILD_MAIN_FILE, [], {
cwd: buildDir,
preferLocal: true,
Expand All @@ -50,6 +53,8 @@ const startPlugin = async function ({ pluginDir, nodePath, buildDir, childEnv })
execPath: nodePath,
env: childEnv,
extendEnv: false,
stdio:
isTrustedPlugin(pluginPackageJson) && systemLogFile ? ['pipe', 'pipe', 'pipe', 'ipc', systemLogFile] : undefined,
})

try {
Expand Down
6 changes: 3 additions & 3 deletions packages/build/src/steps/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { getSuccessStatus } from '../status/success.js'
import { getPluginErrorType } from './error.js'
import { updateNetlifyConfig, listConfigSideFiles } from './update_config.js'

export const isTrustedPlugin = (pluginPackageJson) => pluginPackageJson?.name?.startsWith('@netlify/')

// Fire a plugin step
export const firePluginStep = async function ({
event,
Expand All @@ -33,8 +35,6 @@ export const firePluginStep = async function ({
}) {
const listeners = pipePluginOutput(childProcess, logs)

const isTrustedPlugin = pluginPackageJson?.name?.startsWith('@netlify/')

try {
const configSideFiles = await listConfigSideFiles([headersPath, redirectsPath])
const {
Expand All @@ -48,7 +48,7 @@ export const firePluginStep = async function ({
event,
error,
envChanges,
featureFlags: isTrustedPlugin ? featureFlags : undefined,
featureFlags: isTrustedPlugin(pluginPackageJson) ? featureFlags : undefined,
netlifyConfig,
constants,
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export const onBuild = function ({ featureFlags }) {
export const onBuild = function ({ featureFlags, systemLog }) {
systemLog?.("some system-facing logs")
console.log(JSON.stringify(featureFlags))
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
export const onBuild = function ({ featureFlags }) {
export const onBuild = function ({ featureFlags, systemLog }) {
if (systemLog) {
throw new Error("System log shouldn't be acessible from untrusted plugins")
}
console.log("typeof featureflags:", typeof featureFlags)
}
19 changes: 18 additions & 1 deletion packages/build/tests/plugins/tests.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import * as fs from 'fs/promises'
import { platform } from 'process'
import { fileURLToPath } from 'url'

import { Fixture, normalizeOutput, removeDir } from '@netlify/testing'
import test from 'ava'
import { tmpName } from 'tmp-promise'

const FIXTURES_DIR = fileURLToPath(new URL('fixtures', import.meta.url))

Expand Down Expand Up @@ -142,13 +145,25 @@ test('Plugins can have inputs', async (t) => {
t.snapshot(normalizeOutput(output))
})

test('Plugins are passed featureflags', async (t) => {
test('Trusted plugins are passed featureflags and system log', async (t) => {
const systemLogFile = await tmpName()
const output = await new Fixture('./fixtures/feature_flags')
.withFlags({
featureFlags: { test_flag: true },
debug: false,
systemLogFile: await fs.open(systemLogFile, 'a'),
})
.runWithBuild()

// windows doesn't support the `/dev/fd/` API we're relying on for system logging.
if (platform !== 'win32') {
const systemLog = (await fs.readFile(systemLogFile, { encoding: 'utf8' })).split('\n')

const expectedSystemLogs = 'some system-facing logs'
t.false(output.includes(expectedSystemLogs))
t.true(systemLog.includes(expectedSystemLogs))
}

t.true(
output.includes(
JSON.stringify({
Expand All @@ -165,6 +180,8 @@ test('Plugins are passed featureflags', async (t) => {
const outputUntrusted = await new Fixture('./fixtures/feature_flags_untrusted')
.withFlags({
featureFlags: { test_flag: true },
debug: false,
systemLogFile: await fs.open(systemLogFile, 'a'),
})
.runWithBuild()

Expand Down
1 change: 1 addition & 0 deletions packages/build/types/netlify_plugin_options.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ export interface NetlifyPluginOptions<TInputs extends PluginInputs<StringKeys<TI
constants: NetlifyPluginConstants
utils: NetlifyPluginUtils
featureFlags?: Record<string, unknown>
systemLog?(message: string): void
}

0 comments on commit 8fee92b

Please sign in to comment.