Skip to content

Commit

Permalink
[PWA-680] Webpagetest security score is too low (#2548)
Browse files Browse the repository at this point in the history
* Test some inline definitions to see if this passes the WPT scan

* feat(targets): Add target for editing UPWARD definitions

* - Move security definitions into a new package
- Write an interceptor that injects them into the app shell

* Add new package to docker configs

* Fix to version and add clean script

* Update packages/extensions/upward-security-headers/intercept.js

Co-authored-by: James Zetlen <jzetlen@adobe.com>

* Fix failing test

* Fix for scaffolding to support packages in extensions directory

* Implement PR suggestions

* Fixup failing test

* Add Braintree to CSP rule

Co-authored-by: James Zetlen <jzetlen@adobe.com>
Co-authored-by: Devagouda <40405790+dpatil-magento@users.noreply.github.com>
  • Loading branch information
3 people committed Jul 27, 2020
1 parent 407617e commit 77ab096
Show file tree
Hide file tree
Showing 15 changed files with 190 additions and 41 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Expand Up @@ -18,3 +18,6 @@ docker/certs

## exception: commit node_modules folders in test fixtures
!**/__fixtures__/*/node_modules

## May temporarily create tarballs during scaffolding debug
packages/**/*.tgz
3 changes: 2 additions & 1 deletion dev.dockerfile
Expand Up @@ -17,8 +17,9 @@ RUN apk --no-cache --virtual add \
ENV CI=true

# copy just the dependency files and configs needed for install
COPY packages/create-pwa/package.json ./packages/create-pwa/package.json
COPY packages/babel-preset-peregrine/package.json ./packages/babel-preset-peregrine/package.json
COPY packages/create-pwa/package.json ./packages/create-pwa/package.json
COPY packages/extensions/upward-security-headers/package.json ./packages/extensions/upward-security-headers/package.json
COPY packages/graphql-cli-validate-magento-pwa-queries/package.json ./packages/graphql-cli-validate-magento-pwa-queries/package.json
COPY packages/pagebuilder/package.json ./packages/pagebuilder/package.json
COPY packages/peregrine/package.json ./packages/peregrine/package.json
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -5,6 +5,7 @@
"workspaces": [
"packages/babel-preset-peregrine",
"packages/create-pwa",
"packages/extensions/*",
"packages/graphql-cli-validate-magento-pwa-queries",
"packages/pagebuilder",
"packages/peregrine",
Expand Down
26 changes: 26 additions & 0 deletions packages/extensions/upward-security-headers/intercept.js
@@ -0,0 +1,26 @@
const SECURITY_HEADER_DEFINITION = 'veniaSecurityHeaders';

module.exports = targets => {
const builtins = targets.of('@magento/pwa-buildpack');

builtins.specialFeatures.tap(features => {
features[targets.name] = { upward: true };
});

builtins.transformUpward.tapPromise(async definitions => {
if (!definitions[SECURITY_HEADER_DEFINITION]) {
throw new Error(
`${
targets.name
} could not find its own definition in the emitted upward.yml`
);
}

const shellHeaders = definitions.veniaAppShell.inline.headers.inline;
const securityHeaders = definitions[SECURITY_HEADER_DEFINITION].inline;

for (const name of Object.keys(securityHeaders)) {
shellHeaders[name] = `${SECURITY_HEADER_DEFINITION}.${name}`;
}
});
};
26 changes: 26 additions & 0 deletions packages/extensions/upward-security-headers/package.json
@@ -0,0 +1,26 @@
{
"name": "@magento/upward-security-headers",
"version": "0.0.1",
"publishConfig": {
"access": "public"
},
"description": "Add security headers to UPWARD",
"main": "intercept.js",
"scripts": {
"clean": " "
},
"repository": "github:magento/pwa-studio",
"author": "Magento Commerce",
"license": "(OSL-3.0 OR AFL-3.0)",
"peerDependencies": {
"@magento/pwa-buildpack": "^5.1.1",
"@magento/venia-ui": "^3.0.0",
"rimraf": "~2.6.3",
"webpack": "~4.38.0"
},
"pwa-studio": {
"targets": {
"intercept": "./intercept"
}
}
}
25 changes: 25 additions & 0 deletions packages/extensions/upward-security-headers/upward.yml
@@ -0,0 +1,25 @@
veniaSecurityHeaders:
resolver: inline
inline:
content-security-policy:
resolver: template
engine: mustache
provide:
backend: env.MAGENTO_BACKEND_URL
template:
resolver: conditional
when:
- matches: env.NODE_ENV
pattern: development
use:
inline: ""
default:
inline: "script-src http: https: {{ backend }}; style-src 'self' https: 'unsafe-inline' {{ backend }}; img-src data: http: https:; object-src 'none'; base-uri 'none'; child-src 'self'; font-src 'self' fonts.gstatic.com; frame-src assets.braintreegateway.com"
strict-transport-security:
inline: max-age=31536000
x-content-type-options:
inline: nosniff
x-frame-options:
inline: SAMEORIGIN
x-xss-protection:
inline: '1; mode=block'
42 changes: 41 additions & 1 deletion packages/pwa-buildpack/lib/BuildBus/declare-base.js
Expand Up @@ -138,7 +138,47 @@ module.exports = targets => {
* @param {Object<(string, boolean)>} featureFlags
* @memberof BuiltinTargets
*/
specialFeatures: new targets.types.Sync(['special'])
specialFeatures: new targets.types.Sync(['special']),

/**
* @callback transformUpwardIntercept
* @param {object} Parsed UPWARD definition object.
* @returns {Promise} - Interceptors do not need to return.
*/

/**
* Exposes the fully merged UPWARD definition for fine tuning. The
* UpwardIncludePlugin does a simple shallow merge of the upward.yml
* files in every package which sets the `upward: true` flag in the
* `specialFeatures` object. After that is complete,
* UpwardIncludePlugin calls this target with the parsed and merged
* definition.
*
* @example <caption>Send empty responses in maintenance mode.</caption>
* targets.of('@magento/pwa-buildpack').transformUpward.tap(def => {
* const guardMaintenanceMode = (prop, inline) => {
* def[prop] = {
* when: [
* {
* matches: 'env.MAINTENANCE_MODE',
* pattern: '.',
* use: { inline }
* }
* ],
* default: def[prop]
* }
* }
*
* guardMaintenanceMode('status', 503);
* guardMaintenanceMode('body', '')
* })
*
*
* @type {tapable.AsyncSeriesHook}
* @param {transformUpwardIntercept} interceptor
* @memberof BuiltinTargets
*/
transformUpward: new targets.types.AsyncSeries(['definitions'])
};

/**
Expand Down
Expand Up @@ -33,7 +33,8 @@ async function getClientConfig(opts) {
vendor,
projectConfig,
stats,
resolver
resolver,
bus
} = opts;

let vendorTest = '[\\/]node_modules[\\/]';
Expand Down Expand Up @@ -82,6 +83,7 @@ async function getClientConfig(opts) {
}),
new webpack.EnvironmentPlugin(projectConfig.env),
new UpwardIncludePlugin({
bus,
upwardDirs: [...hasFlag('upward'), context]
}),
new WebpackAssetsManifest({
Expand Down
Expand Up @@ -10,7 +10,8 @@ const jsYaml = require('js-yaml');
* autodetects file assets relied on by those configurations
*/
class UpwardIncludePlugin {
constructor({ upwardDirs }) {
constructor({ bus, upwardDirs }) {
this.bus = bus;
this.upwardDirs = upwardDirs;
this.definition = {};
debug('created with dirs: %s', upwardDirs);
Expand All @@ -33,7 +34,12 @@ class UpwardIncludePlugin {
context,
from: './upward.yml',
to: './upward.yml',
transform: () => jsYaml.safeDump(this.definition)
transform: async () => {
await this.bus
.getTargetsOf('@magento/pwa-buildpack')
.transformUpward.promise(this.definition);
return jsYaml.safeDump(this.definition);
}
}
};
this.dirs = new Set([...this.upwardDirs, context]);
Expand Down
Expand Up @@ -2,6 +2,8 @@ const { join } = require('path');
const MemoryFS = require('memory-fs');
const webpack = require('webpack');
const jsYaml = require('js-yaml');

const { mockBuildBus } = require('../../../TestHelpers');
const UpwardIncludePlugin = require('../UpwardIncludePlugin');

const basic3PageProjectDir = join(
Expand Down Expand Up @@ -31,6 +33,12 @@ const compile = config =>
});

test('merges upward files and resources', async () => {
const bus = mockBuildBus({
context: __dirname,
dependencies: [{ name: '@magento/pwa-buildpack' }]
});
bus.runPhase('declare');

const config = {
context: basic1PageProjectDir,
entry: {
Expand All @@ -41,6 +49,7 @@ test('merges upward files and resources', async () => {
},
plugins: [
new UpwardIncludePlugin({
bus,
upwardDirs: [basic3PageProjectDir, basic1PageProjectDir]
})
]
Expand Down
9 changes: 0 additions & 9 deletions packages/pwa-buildpack/lib/cli/create-project.js
Expand Up @@ -219,15 +219,6 @@ module.exports.handler = async function buildpackCli(argv) {
prettyLogger.success(`Installed dependencies for '${name}' project`);
}

if (process.env.DEBUG_PROJECT_CREATION) {
prettyLogger.info('Debug: Removing generated tarballs');
const pkgDir = require('pkg-dir');
const monorepoDir = resolve(pkgDir.sync(__dirname), '../../');
prettyLogger.info(
execa.shellSync('rm -v packages/*/*.tgz', { cwd: monorepoDir })
.stdout
);
}
const showCommand = command =>
' - ' + chalk.whiteBright(`${params.npmClient} ${command}`);
const buildpackPrefix = params.npmClient === 'npm' ? ' --' : '';
Expand Down
13 changes: 10 additions & 3 deletions packages/venia-concept/_buildpack/__tests__/create.spec.js
Expand Up @@ -182,13 +182,20 @@ test('forces yarn client, local deps, and console debugging if DEBUG_PROJECT_CRE
}),
[resolve(packagesRoot, 'peregrine/package.json')]: JSON.stringify({
name: '@magento/peregrine'
}),
[resolve(packagesRoot, 'bad-package/package.json')]: 'bad json',
[resolve(packagesRoot, 'some-file.txt')]: 'not a package'
})
};

const fs = mockFs(files);

// mock the yarn workspaces response
execSync.mockReturnValueOnce(
JSON.stringify({
foo: { location: '/repo/packages/me' },
'@magento/peregrine': { location: 'packages/peregrine' },
'@magento/venia-ui': { location: 'packages/venia-ui' }
})
);

await runCreate(fs, {
name: 'foo',
author: 'bar',
Expand Down
56 changes: 33 additions & 23 deletions packages/venia-concept/_buildpack/create.js
Expand Up @@ -132,35 +132,45 @@ function setDebugDependencies(fs, pkg) {
console.warn(
'DEBUG_PROJECT_CREATION: Debugging Venia _buildpack/create.js, so we will assume we are inside the pwa-studio repo and replace those package dependency declarations with local file paths.'
);

const { execSync } = require('child_process');
const overridden = {};
const workspaceDir = resolve(__dirname, '../../');
fs.readdirSync(workspaceDir).forEach(packageDir => {
const packagePath = resolve(workspaceDir, packageDir);
if (!fs.statSync(packagePath).isDirectory()) {
return;
}
let name;
try {
name = fs.readJsonSync(resolve(packagePath, 'package.json')).name;
} catch (e) {} // eslint-disable-line no-empty
if (
// these should not be deps
!name ||
name === '@magento/create-pwa' ||
name === '@magento/venia-concept'
) {
const monorepoDir = resolve(__dirname, '../../../');

// The Yarn "workspaces info" command outputs JSON as of v1.22.4.
// The -s flag suppresses all other non-JSON logging output.
const yarnWorkspaceInfoCmd = 'yarn -s workspaces info';
const workspaceInfo = execSync(yarnWorkspaceInfoCmd, { cwd: monorepoDir });

let packageDirs;
try {
packageDirs = Object.values(JSON.parse(workspaceInfo)).map(
({ location }) => resolve(monorepoDir, location)
);
} catch (e) {
throw new Error(
`DEBUG_PROJECT_CREATION: Could not parse output of '${yarnWorkspaceInfoCmd}:\n${workspaceInfo}. Please check your version of yarn is v1.22.4+.`
);
}

packageDirs.forEach(packageDir => {
const name = fs.readJsonSync(resolve(packageDir, 'package.json')).name;
const packagesToSkip = [
'@magento/create-pwa',
'@magento/venia-concept'
];

if (packagesToSkip.includes(name)) {
return;
}

console.warn(`DEBUG_PROJECT_CREATION: Packing ${name} for local usage`);
let filename;
let packOutput;
try {
packOutput = require('child_process').execSync(
'npm pack -s --ignore-scripts --json',
{
cwd: packagePath
}
);
packOutput = execSync('npm pack -s --ignore-scripts --json', {
cwd: packageDir
});
filename = JSON.parse(packOutput)[0].filename;
} catch (e) {
throw new Error(
Expand All @@ -169,7 +179,7 @@ function setDebugDependencies(fs, pkg) {
}`
);
}
const localDep = `file://${resolve(packagePath, filename)}`;
const localDep = `file://${resolve(packageDir, filename)}`;
['dependencies', 'devDependencies', 'optionalDependencies'].forEach(
depType => {
if (pkg[depType] && pkg[depType][name]) {
Expand Down
1 change: 1 addition & 0 deletions packages/venia-concept/package.json
Expand Up @@ -52,6 +52,7 @@
"@magento/peregrine": "~6.0.0",
"@magento/pwa-buildpack": "~5.1.1",
"@magento/upward-js": "~4.0.1",
"@magento/upward-security-headers": "~0.0.1",
"@magento/venia-ui": "~3.0.0",
"@storybook/react": "~5.2.6",
"apollo-cache-inmemory": "~1.6.3",
Expand Down
3 changes: 2 additions & 1 deletion prod.dockerfile
Expand Up @@ -13,8 +13,9 @@ RUN apk --no-cache --virtual add \
ENV CI=true

# copy just the dependency files and configs needed for install
COPY packages/create-pwa/package.json ./packages/create-pwa/package.json
COPY packages/babel-preset-peregrine/package.json ./packages/babel-preset-peregrine/package.json
COPY packages/create-pwa/package.json ./packages/create-pwa/package.json
COPY packages/extensions/upward-security-headers/package.json ./packages/extensions/upward-security-headers/package.json
COPY packages/graphql-cli-validate-magento-pwa-queries/package.json ./packages/graphql-cli-validate-magento-pwa-queries/package.json
COPY packages/pagebuilder/package.json ./packages/pagebuilder/package.json
COPY packages/peregrine/package.json ./packages/peregrine/package.json
Expand Down

0 comments on commit 77ab096

Please sign in to comment.