From c450d8d5552f4379c23d5f72058cec2f9bec86cf Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Mon, 28 Sep 2020 09:44:14 -0700 Subject: [PATCH 1/4] Send CSRF tokens over XHR (#15778) * Send CSRF tokens over XHR * Update events.js * Update browser.js --- includes/head.html | 10 ----- javascripts/get-csrf.js | 14 +++++++ javascripts/index.js | 2 + middleware/context.js | 8 ---- middleware/csrf-route.js | 2 +- middleware/csrf.js | 3 +- tests/browser/browser.js | 7 ++++ tests/rendering/csrf-route.js | 2 +- tests/rendering/events.js | 74 ++++++++++++++++++++++++++--------- 9 files changed, 81 insertions(+), 41 deletions(-) diff --git a/includes/head.html b/includes/head.html index a398a7932b..30c08fbd81 100644 --- a/includes/head.html +++ b/includes/head.html @@ -25,14 +25,4 @@ - - {% if page.relativePath contains "managing-your-work-on-github/disabling-project-boards-in-your-organization" %} - - {% if fastlyEnabled %} - - - {% else %} - - {% endif %} - {% endif %} diff --git a/javascripts/get-csrf.js b/javascripts/get-csrf.js index 825dce25cb..a2ea86db46 100644 --- a/javascripts/get-csrf.js +++ b/javascripts/get-csrf.js @@ -1,3 +1,17 @@ +export async function fillCsrf () { + const response = await fetch('/csrf', { + method: 'GET', + headers: { + 'Content-Type': 'application/json' + } + }) + const json = response.ok ? await response.json() : {} + const meta = document.createElement('meta') + meta.setAttribute('name', 'csrf-token') + meta.setAttribute('content', json.token) + document.querySelector('head').append(meta) +} + export default function getCsrf () { const csrfEl = document .querySelector('meta[name="csrf-token"]') diff --git a/javascripts/index.js b/javascripts/index.js index d88b054bdc..7eda77be37 100644 --- a/javascripts/index.js +++ b/javascripts/index.js @@ -13,6 +13,7 @@ import print from './print' import localization from './localization' import helpfulness from './helpfulness' import experiment from './experiment' +import { fillCsrf } from './get-csrf' document.addEventListener('DOMContentLoaded', () => { displayPlatformSpecificContent() @@ -26,6 +27,7 @@ document.addEventListener('DOMContentLoaded', () => { wrapCodeTerms() print() localization() + fillCsrf() helpfulness() experiment() }) diff --git a/middleware/context.js b/middleware/context.js index 4160f22c38..9b44ca2c9d 100644 --- a/middleware/context.js +++ b/middleware/context.js @@ -55,14 +55,6 @@ module.exports = async function contextualize (req, res, next) { req.context.siteTree = siteTree req.context.pages = pages - // To securely accept data from end users, - // we need to validate that they were actually on a docs page first. - // We'll put this token in the and call on it when we send user data - // to the docs server, so we know the request came from someone on the page. - req.context.csrfToken = req.csrfToken() - req.context.fastlyEnabled = process.env.NODE_ENV === 'production' && - req.hostname === 'docs.github.com' - return next() } diff --git a/middleware/csrf-route.js b/middleware/csrf-route.js index eea25b3acc..c884f294d8 100644 --- a/middleware/csrf-route.js +++ b/middleware/csrf-route.js @@ -8,7 +8,7 @@ router.get('/', (req, res) => { 'surrogate-control': 'private, no-store', 'cache-control': 'private, no-store' }) - res.send(``) + res.json({ token: req.csrfToken() }) }) module.exports = router diff --git a/middleware/csrf.js b/middleware/csrf.js index 6110558ad7..914546178b 100644 --- a/middleware/csrf.js +++ b/middleware/csrf.js @@ -1,5 +1,4 @@ module.exports = require('csurf')({ cookie: require('../lib/cookie-settings'), - ignoreMethods: ['GET', 'HEAD', 'OPTIONS', 'POST', 'PUT'] - // TODO CSRF edit this to include POST and PUT to require it + ignoreMethods: ['GET', 'HEAD', 'OPTIONS'] }) diff --git a/tests/browser/browser.js b/tests/browser/browser.js index 72e14aac09..bc133e09f8 100644 --- a/tests/browser/browser.js +++ b/tests/browser/browser.js @@ -135,6 +135,13 @@ describe('helpfulness', () => { }) }) +describe('csrf meta', () => { + it('should have a csrf-token meta tag on the page', async () => { + await page.goto('http://localhost:4001/en/actions/getting-started-with-github-actions/about-github-actions') + await page.waitForSelector('meta[name="csrf-token"]') + }) +}) + async function getLocationObject (page) { const location = await page.evaluate(() => { return Promise.resolve(JSON.stringify(window.location, null, 2)) diff --git a/tests/rendering/csrf-route.js b/tests/rendering/csrf-route.js index 2ef6a0aeb1..62609e7a1d 100644 --- a/tests/rendering/csrf-route.js +++ b/tests/rendering/csrf-route.js @@ -9,7 +9,7 @@ describe('GET /csrf', () => { it('should render a non-cache include for CSRF token', async () => { const res = await request(app).get('/csrf') expect(res.status).toBe(200) - expect(res.text).toMatch(/^$/) + expect(res.body).toHaveProperty('token') expect(res.headers['surrogate-control']).toBe('private, no-store') expect(res.headers['cache-control']).toBe('private, no-store') }) diff --git a/tests/rendering/events.js b/tests/rendering/events.js index 5b9de0c1e3..0380ca09ab 100644 --- a/tests/rendering/events.js +++ b/tests/rendering/events.js @@ -11,14 +11,23 @@ Airtable.mockImplementation(function () { }) describe('POST /events', () => { - beforeEach(() => { + jest.setTimeout(60 * 1000) + + let csrfToken = '' + let agent + + beforeEach(async () => { process.env.AIRTABLE_API_KEY = '$AIRTABLE_API_KEY$' process.env.AIRTABLE_BASE_KEY = '$AIRTABLE_BASE_KEY$' + agent = request.agent(app) + const csrfRes = await agent.get('/csrf') + csrfToken = csrfRes.body.token }) afterEach(() => { delete process.env.AIRTABLE_API_KEY delete process.env.AIRTABLE_BASE_KEY + csrfToken = '' }) describe('HELPFULNESS', () => { @@ -32,82 +41,93 @@ describe('POST /events', () => { } it('should accept a valid object', () => - request(app) + agent .post('/events') .send(example) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(201) ) it('should reject extra properties', () => - request(app) + agent .post('/events') .send({ ...example, toothpaste: false }) .set('Accept', 'application/json') + + .set('csrf-token', csrfToken) .expect(400) ) it('should not accept if type is missing', () => - request(app) + agent .post('/events') .send({ ...example, type: undefined }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should not accept if url is missing', () => - request(app) + agent .post('/events') .send({ ...example, url: undefined }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should not accept if url is misformatted', () => - request(app) + agent .post('/events') .send({ ...example, url: 'examplecom' }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should not accept if vote is missing', () => - request(app) + agent .post('/events') .send({ ...example, vote: undefined }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should not accept if vote is not boolean', () => - request(app) + agent .post('/events') .send({ ...example, vote: 'true' }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should not accept if email is misformatted', () => - request(app) + agent .post('/events') .send({ ...example, email: 'testexample.com' }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should not accept if comment is not string', () => - request(app) + agent .post('/events') .send({ ...example, comment: [] }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should not accept if category is not an option', () => - request(app) + agent .post('/events') .send({ ...example, category: 'Fabulous' }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) }) @@ -122,64 +142,79 @@ describe('POST /events', () => { } it('should accept a valid object', () => - request(app) + agent .post('/events') .send(example) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(201) ) it('should reject extra fields', () => - request(app) + agent .post('/events') .send({ ...example, toothpaste: false }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should require a long unique user-id', () => - request(app) + agent .post('/events') .send({ ...example, 'user-id': 'short' }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should require a test', () => - request(app) + agent .post('/events') .send({ ...example, test: undefined }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should require a valid group', () => - request(app) + agent .post('/events') .send({ ...example, group: 'revolution' }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(400) ) it('should default the success field', () => - request(app) + agent .post('/events') .send({ ...example, success: undefined }) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(201) ) }) }) describe('PUT /events/:id', () => { - beforeEach(() => { + jest.setTimeout(60 * 1000) + + let csrfToken = '' + let agent + + beforeEach(async () => { process.env.AIRTABLE_API_KEY = '$AIRTABLE_API_KEY$' process.env.AIRTABLE_BASE_KEY = '$AIRTABLE_BASE_KEY$' + agent = request.agent(app) + const csrfRes = await agent.get('/csrf') + csrfToken = csrfRes.body.token }) afterEach(() => { delete process.env.AIRTABLE_API_KEY delete process.env.AIRTABLE_BASE_KEY + csrfToken = '' }) const example = { @@ -192,10 +227,11 @@ describe('PUT /events/:id', () => { } it('should update an existing HELPFULNESS event', () => - request(app) + agent .put('/events/TESTID') .send(example) .set('Accept', 'application/json') + .set('csrf-token', csrfToken) .expect(200) ) }) From 5d5c688c7cb81c5819bf1cc5a8794132be3ba8da Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Mon, 28 Sep 2020 10:06:04 -0700 Subject: [PATCH 2/4] Set doctype html on all pages (#15779) --- includes/head.html | 1 + layouts/default.html | 1 + layouts/product-landing.html | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/includes/head.html b/includes/head.html index 30c08fbd81..3eba2d1610 100644 --- a/includes/head.html +++ b/includes/head.html @@ -1,4 +1,5 @@ + {% if error == '404' %}{{ site.data.ui.errors.oops }}{% elsif currentVersion == 'homepage' %}GitHub Documentation{% else %}{{ page.fullTitle }}{% endif %} {% if page.hidden %} {% endif %} diff --git a/layouts/default.html b/layouts/default.html index 3981947a25..cbb745a516 100644 --- a/layouts/default.html +++ b/layouts/default.html @@ -1,3 +1,4 @@ + {% include head %} diff --git a/layouts/product-landing.html b/layouts/product-landing.html index a4823ec06d..5bd1189b1a 100644 --- a/layouts/product-landing.html +++ b/layouts/product-landing.html @@ -1,9 +1,10 @@ + {% include head %} {% include sidebar %} - +
{% include header %} From ae0d0f530eb793a44cd005e5bf2ae85fd49ffb89 Mon Sep 17 00:00:00 2001 From: Shati Patel <42641846+shati-patel@users.noreply.github.com> Date: Mon, 28 Sep 2020 18:42:34 +0100 Subject: [PATCH 3/4] Code scanning troubleshooting: How to set up environment with CodeQL runner (#15746) * Code scanning troubleshooting: Set up environment with CodeQL runner * Change "trace" to "monitor" * Fix note syntax --- ...running-code-scanning-in-your-ci-system.md | 4 ++-- ...hooting-code-scanning-in-your-ci-system.md | 24 +++++++++++++++++-- .../troubleshooting-the-codeql-workflow.md | 2 +- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/content/github/finding-security-vulnerabilities-and-errors-in-your-code/running-code-scanning-in-your-ci-system.md b/content/github/finding-security-vulnerabilities-and-errors-in-your-code/running-code-scanning-in-your-ci-system.md index c280d89869..f22e4fb2b2 100644 --- a/content/github/finding-security-vulnerabilities-and-errors-in-your-code/running-code-scanning-in-your-ci-system.md +++ b/content/github/finding-security-vulnerabilities-and-errors-in-your-code/running-code-scanning-in-your-ci-system.md @@ -97,7 +97,7 @@ The server has access to download the {{ site.data.variables.product.prodname_co #### Compiled language example -This example is similar to the previous example, however this time the repository has code in C/C++, C#, or Java. To create a {{ site.data.variables.product.prodname_codeql }} database for these languages, the CLI needs to trace the build. At the end of the initialization process, the runner reports the command you need to set up the environment before building the code. You need to run this command, before calling the normal CI build process, and then running the `analyze` command. +This example is similar to the previous example, however this time the repository has code in C/C++, C#, or Java. To create a {{ site.data.variables.product.prodname_codeql }} database for these languages, the CLI needs to monitor the build. At the end of the initialization process, the runner reports the command you need to set up the environment before building the code. You need to run this command, before calling the normal CI build process, and then running the `analyze` command. 1. Check out the repository to analyze. 1. Move into the directory where the repository is checked out. @@ -114,7 +114,7 @@ This example is similar to the previous example, however this time the repositor . /srv/checkout/example-repo-2/codeql-runner/codeql-env.sh". ``` -1. Run the script generated by the `init` action to set up the environment to trace the build. +1. Run the script generated by the `init` action to set up the environment to monitor the build. ```shell $ . /srv/checkout/example-repo-2/codeql-runner/codeql-env.sh diff --git a/content/github/finding-security-vulnerabilities-and-errors-in-your-code/troubleshooting-code-scanning-in-your-ci-system.md b/content/github/finding-security-vulnerabilities-and-errors-in-your-code/troubleshooting-code-scanning-in-your-ci-system.md index 986c17a0a4..e10d6f6719 100644 --- a/content/github/finding-security-vulnerabilities-and-errors-in-your-code/troubleshooting-code-scanning-in-your-ci-system.md +++ b/content/github/finding-security-vulnerabilities-and-errors-in-your-code/troubleshooting-code-scanning-in-your-ci-system.md @@ -22,10 +22,30 @@ To avoid this automatic download, you can manually download the {{ site.data.var ### No code found during the build -If the `analyze` command for the {{ site.data.variables.product.prodname_codeql_runner }} fails with an error `No source code was seen during the build`, this indicates that {{ site.data.variables.product.prodname_codeql }} was unable to trace your code. Several reasons can explain such a failure. +If the `analyze` command for the {{ site.data.variables.product.prodname_codeql_runner }} fails with an error `No source code was seen during the build`, this indicates that {{ site.data.variables.product.prodname_codeql }} was unable to monitor your code. Several reasons can explain such a failure. 1. Automatic language detection identified a supported language, but there is no analyzable code of that language in the repository. A typical example is when our language detection service finds a file associated with a particular programming language like a `.h`, or `.gyp` file, but no corresponding executable code is present in the repository. To solve the problem, you can manually define the languages you want to analyze by using the `--languages` flag of the `init` command. For more information, see "[Configuring {{ site.data.variables.product.prodname_code_scanning }} in your CI system](/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-code-scanning-in-your-ci-system)." -1. You're analyzing a compiled language without using the `autobuild` command and you run the build steps yourself after the `init` step. For the build to work, you must set up the environment such that the {{ site.data.variables.product.prodname_codeql_runner }} can trace the code. The `init` command generates instructions for how to export the required environment variables, so you can copy and run the script. For more information, see "[Running {{ site.data.variables.product.prodname_code_scanning }} in your CI system](/github/finding-security-vulnerabilities-and-errors-in-your-code/running-code-scanning-in-your-ci-system#compiled-language-example)." +1. You're analyzing a compiled language without using the `autobuild` command and you run the build steps yourself after the `init` step. For the build to work, you must set up the environment such that the {{ site.data.variables.product.prodname_codeql_runner }} can monitor the code. The `init` command generates instructions for how to export the required environment variables, so you can copy and run the script after you've run the `init` command. + - On macOS and Linux: + ```shell + $ . codeql-runner/codeql-env.sh + ``` + - On Windows, using the Command shell (`cmd`) or a batch file (`.bat`): + ```shell + > call codeql-runner\codeql-env.bat + ``` + - On Windows, using PowerShell: + ```shell + > cat codeql-runner\codeql-env.sh | Invoke-Expression + ``` + + The environment variables are also stored in the file `codeql-runner/codeql-env.json`. This file contains a single JSON object which maps environment variable keys to values. If you can't run the script generated by the `init` command, then you can use the data in JSON format instead. + + {% note %} + + **Note:** If you used the `--temp-dir` flag of the `init` command to specify a custom directory for temporary files, the path to the `codeql-env` files might be different. + + {% endnote %} 1. The code is built in a container or on a separate machine. If you use a containerized build or if you outsource the build to another machine, make sure to run the {{ site.data.variables.product.prodname_codeql_runner }} in the container or on the machine where your build task takes place. diff --git a/content/github/finding-security-vulnerabilities-and-errors-in-your-code/troubleshooting-the-codeql-workflow.md b/content/github/finding-security-vulnerabilities-and-errors-in-your-code/troubleshooting-the-codeql-workflow.md index 6e0d0b95dc..601b000049 100644 --- a/content/github/finding-security-vulnerabilities-and-errors-in-your-code/troubleshooting-the-codeql-workflow.md +++ b/content/github/finding-security-vulnerabilities-and-errors-in-your-code/troubleshooting-the-codeql-workflow.md @@ -44,7 +44,7 @@ If an automatic build of code for a compiled language within your project fails, ### No code found during the build -If your workflow fails with an error `No source code was seen during the build` or `The process '/opt/hostedtoolcache/CodeQL/0.0.0-20200630/x64/codeql/codeql' failed with exit code 32`, this indicates that {{ site.data.variables.product.prodname_codeql }} was unable to trace your code. Several reasons can explain such a failure: +If your workflow fails with an error `No source code was seen during the build` or `The process '/opt/hostedtoolcache/CodeQL/0.0.0-20200630/x64/codeql/codeql' failed with exit code 32`, this indicates that {{ site.data.variables.product.prodname_codeql }} was unable to monitor your code. Several reasons can explain such a failure: 1. Automatic language detection identified a supported language, but there is no analyzable code of that language in the repository. A typical example is when our language detection service finds a file associated with a particular programming language like a `.h`, or `.gyp` file, but no corresponding executable code is present in the repository. To solve the problem, you can manually define the languages you want to analyze by updating the list of languages in the `language` matrix. For example, the following configuration will analyze only Go, and JavaScript. From a302fbf36a920296aef564618ec9b43478c40529 Mon Sep 17 00:00:00 2001 From: Shati Patel <42641846+shati-patel@users.noreply.github.com> Date: Mon, 28 Sep 2020 19:32:02 +0100 Subject: [PATCH 4/4] Code scanning: autobuild update on Windows and macOS (#15078) [Sept 22 or later] Code scanning: C++ autobuild for Windows and macOS --- ...the-codeql-workflow-for-compiled-languages.md | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/content/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-the-codeql-workflow-for-compiled-languages.md b/content/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-the-codeql-workflow-for-compiled-languages.md index 5ad587e051..667dad88ae 100644 --- a/content/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-the-codeql-workflow-for-compiled-languages.md +++ b/content/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-the-codeql-workflow-for-compiled-languages.md @@ -40,10 +40,16 @@ If your workflow uses a `language` matrix, `autobuild` attempts to build each of | Supported system type | System name | |----|----| -| Operating system | Windows and Linux | -| Build system | Autoconf, CMake, qmake, Meson, Waf, SCons, and Linux Kbuild | +| Operating system | Windows, macOS, and Linux | +| Build system | Windows: MSbuild and build scripts
Linux and macOS: Autoconf, Make, CMake, qmake, Meson, Waf, SCons, Linux Kbuild, and build scripts | -The behavior of the `autobuild` step varies according to the operating system that the extraction runs on. On Windows, the step has no default actions. On Linux, this step reviews the files present in the repository to determine the build system used: +The behavior of the `autobuild` step varies according to the operating system that the extraction runs on. On Windows, the `autobuild` step attempts to autodetect a suitable build method for C/C++ using the following approach: + +1. Invoke `MSBuild.exe` on the solution (`.sln`) or project (`.vcxproj`) file closest to the root. +If `autobuild` detects multiple solution or project files at the same (shortest) depth from the top level directory, it will attempt to build all of them. +2. Invoke a script that looks like a build script—_build.bat_, _build.cmd_, _and build.exe_ (in that order). + +On Linux and macOS, the `autobuild` step reviews the files present in the repository to determine the build system used: 1. Look for a build system in the root directory. 2. If none are found, search subdirectories for a unique directory with a build system for C/C++. @@ -53,7 +59,7 @@ The behavior of the `autobuild` step varies according to the operating system th | Supported system type | System name | |----|----| -| Operating system | Windows and Linux | +| Operating system | Windows and Linux | | Build system | .NET and MSbuild, as well as build scripts | The `autobuild` process attempts to autodetect a suitable build method for C# using the following approach: @@ -67,7 +73,7 @@ If `autobuild` detects multiple solution or project files at the same (shortest) | Supported system type | System name | |----|----| -| Operating system | Windows, macOS and Linux (no restriction) | +| Operating system | Windows, macOS, and Linux (no restriction) | | Build system | Gradle, Maven and Ant | The `autobuild` process tries to determine the build system for Java codebases by applying this strategy: