Skip to content

Commit

Permalink
Add pre/post build compliance templates and address PoliCheck issues (#…
Browse files Browse the repository at this point in the history
…10468)

## Description

This PR refactors some of our compliance-related tasks such as CredScan, PoliCheck, and Component Governance into two new templates: `run-compliance-prebuild.yml` and `run-compliance-postbuild.yml`.

The pre-build tasks will run before CI, PR, Publish, and Compliance pipelines. Task failures will cause the CI, PR, and Publish pipelines to fail appropriately. The Compliance pipeline will convert errors into warnings (so that all of the tasks still run).

In addition, this PR address existing PoliCheck issues (so that the PR passes).

### Type of Change
- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)

### Why
Make sure we're running compliance tasks correctly, consistently, and address existing violations.

Closes #10459

### What
Refactors some tasks into new templates and fixes comments in some files.

## Screenshots
N/A

## Testing

For the new pipeline templates, running successfully in this PR.

For the PoliCheck they're all just in comments, so if the PR checks pass, they've been resolved.
  • Loading branch information
jonthysell committed Aug 31, 2022
1 parent 6363177 commit 0512b19
Show file tree
Hide file tree
Showing 14 changed files with 111 additions and 77 deletions.
62 changes: 11 additions & 51 deletions .ado/compliance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ parameters:
displayName: Force CodeQL to rebuild databases
type: boolean
default: false
- name: breakOnComplianceFailure
displayName: Break build on a compliance task failure
- name: complianceWarnOnly
displayName: Convert compliance errors to warnings
type: boolean
default: true
default: true # Let's get all results in this pipeline

variables:
- template: variables/windows.yml
Expand Down Expand Up @@ -59,52 +59,15 @@ jobs:

# Pre-build compliance tasks

# PoliCheck Build Task (https://aka.ms/gdn-azdo-policheck)
# Scans the text of source code, comments, and content for terminology that could be sensitive for legal, cultural, or geopolitical reasons.
- task: securedevelopmentteam.vss-secure-development-tools.build-task-policheck.PoliCheck@2
displayName: '⚖️ Run PoliCheck'
inputs:
targetType: F
targetArgument: $(Build.SourcesDirectory)
result: PoliCheck.xml
optionsFC: 1
optionsXS: 1
optionsHMENABLE: 0
optionsPE: 1|2|3|4
optionsSEV: 1|2|3|4
optionsUEPath: $(Build.SourcesDirectory)\.ado\config\PoliCheckExclusions.xml
optionsRulesDBPath: $(Build.SourcesDirectory)\.ado\config\PoliCheckRules.mdb
continueOnError: true

# CredScan Task (https://aka.ms/gdn-azdo-credscan)
# Searches through source code and build outputs for credentials left behind in the open.
- task: securedevelopmentteam.vss-secure-development-tools.build-task-credscan.CredScan@3
displayName: '⚖️ Run CredScan'
inputs:
outputFormat: pre
suppressionsFile: $(Build.SourcesDirectory)\.ado\config\CredScanSuppressions.json
batchSize: 20
debugMode: false
continueOnError: true

# PostAnalysis Task (https://docs.microsoft.com/en-us/azure/security/develop/yaml-configuration#post-analysis-task)
# Breaks the build if any of the tasks failed.
- task: PostAnalysis@1
displayName: "⚖️ Post Analysis"
inputs:
AllTools: false
CredScan: ${{ parameters.breakOnComplianceFailure }}
PoliCheck: ${{ parameters.breakOnComplianceFailure }}
PoliCheckBreakOn: Severity4Above
ToolLogsNotFoundAction: "Error"
continueOnError: true
- template: templates/run-compliance-prebuild.yml
parameters:
complianceWarnOnly: ${{ parameters.complianceWarnOnly }}

# Initialize CodeQL 3000 Task (https://aka.ms/codeql3000)
# Performs static code analysis.
# GitHub repos not supported, see issue #9994.
- task: CodeQL3000Init@0
displayName: "🛡️ Initialize CodeQL"
continueOnError: true
continueOnError: ${{ parameters.complianceWarnOnly }}

# Build RNW

Expand All @@ -117,15 +80,12 @@ jobs:

# Post-build compliance tasks

# Component Governance Detection Task (https://docs.opensource.microsoft.com/tools/cg/)
# Detects open source you use and alerts you to whether it has security vulnerabilities or legal issues.
- task: ms.vss-governance-buildtask.governance-build-task-component-detection.ComponentGovernanceComponentDetection@0
displayName: 'Component Detection'
continueOnError: true
- template: templates/run-compliance-postbuild.yml
parameters:
complianceWarnOnly: ${{ parameters.complianceWarnOnly }}

# Finalize CodeQL 3000 Task (https://aka.ms/codeql3000)
# Performs static code analysis.
# GitHub repos not supported, see issue #9994.
- task: CodeQL3000Finalize@0
displayName: "🛡️ Finalize CodeQL"
continueOnError: true
continueOnError: ${{ parameters.complianceWarnOnly }}
Binary file modified .ado/config/PoliCheckRules.mdb
Binary file not shown.
7 changes: 2 additions & 5 deletions .ado/jobs/linting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,10 @@ jobs:
steps:
- template: ../templates/checkout-shallow.yml

- task: securedevelopmentteam.vss-secure-development-tools.build-task-credscan.CredScan@3
displayName: '⚖️ Run CredScan'
inputs:
suppressionsFile: $(Build.SourcesDirectory)\.ado\config\CredScanSuppressions.json

- template: ../templates/prepare-js-env.yml

- template: ../templates/run-compliance-prebuild.yml

- script: yarn format:verify
displayName: yarn format:verify

Expand Down
7 changes: 2 additions & 5 deletions .ado/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,6 @@ jobs:
steps:
- template: templates/checkout-full.yml

- task: securedevelopmentteam.vss-secure-development-tools.build-task-credscan.CredScan@3
displayName: '⚖️ Run CredScan'
inputs:
suppressionsFile: $(Build.SourcesDirectory)\.ado\config\CredScanSuppressions.json

- powershell: gci env:/BUILD_*
displayName: Show build information

Expand All @@ -68,6 +63,8 @@ jobs:
displayName: Stop pipeline if latest commit message contains ***NO_CI***
condition: and(${{ parameters.stopOnNoCI }}, contains(variables['Build.SourceVersionMessage'], '***NO_CI***'))
- template: templates/run-compliance-prebuild.yml

- script: |
echo ##vso[task.setvariable variable=SkipNpmPublishArgs]--no-publish
displayName: Enable No-Publish
Expand Down
14 changes: 14 additions & 0 deletions .ado/templates/run-compliance-postbuild.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Compliance tasks to be run post build
parameters:
- name: complianceWarnOnly
displayName: Convert compliance errors to warnings
type: boolean
default: false

steps:
# Component Governance Detection Task (https://docs.opensource.microsoft.com/tools/cg/)
# Detects open source you use and alerts you to whether it has security vulnerabilities or legal issues.
# TODO: Reconcile with existing component-governance.yml template
- task: ms.vss-governance-buildtask.governance-build-task-component-detection.ComponentGovernanceComponentDetection@0
displayName: '⚖️ Component Governance Detection'
continueOnError: ${{ parameters.complianceWarnOnly }}
52 changes: 52 additions & 0 deletions .ado/templates/run-compliance-prebuild.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Compliance tasks to be run pre build
parameters:
- name: complianceWarnOnly
displayName: Convert compliance errors to warnings
type: boolean
default: false

steps:
# PoliCheck Build Task (https://aka.ms/gdn-azdo-policheck)
# Scans the text of source code, comments, and content for terminology that could be sensitive for legal, cultural, or geopolitical reasons.
- task: securedevelopmentteam.vss-secure-development-tools.build-task-policheck.PoliCheck@2
displayName: '⚖️ Run PoliCheck'
inputs:
targetType: F
targetArgument: $(Build.SourcesDirectory)
result: PoliCheck.xml
optionsFC: 1
optionsXS: 1
optionsHMENABLE: 0
optionsPE: 1|2|3|4
optionsSEV: 1|2|3|4
optionsUEPath: $(Build.SourcesDirectory)\.ado\config\PoliCheckExclusions.xml
optionsRulesDBPath: $(Build.SourcesDirectory)\.ado\config\PoliCheckRules.mdb
continueOnError: ${{ parameters.complianceWarnOnly }}

# CredScan Task (https://aka.ms/gdn-azdo-credscan)
# Searches through source code and build outputs for credentials left behind in the open.
- task: securedevelopmentteam.vss-secure-development-tools.build-task-credscan.CredScan@3
displayName: '⚖️ Run CredScan'
inputs:
outputFormat: pre
suppressionsFile: $(Build.SourcesDirectory)\.ado\config\CredScanSuppressions.json
batchSize: 20
debugMode: false
continueOnError: ${{ parameters.complianceWarnOnly }}

# PostAnalysis Task (https://docs.microsoft.com/en-us/azure/security/develop/yaml-configuration#post-analysis-task)
# Breaks the build if any of the tasks failed.
- task: PostAnalysis@1
displayName: "⚖️ Compliance Pre-Build Analysis"
inputs:
AllTools: false
CredScan: true
PoliCheck: true
PoliCheckBreakOn: Severity4Above
ToolLogsNotFoundAction: "Error"
continueOnError: ${{ parameters.complianceWarnOnly }}

# Restore unnecessary changes that were made by the compliance tasks
- script: |
git restore $(Build.SourcesDirectory)\.ado\config\PoliCheckRules.mdb
displayName: "⚖️ Compliance Pre-Build Cleanup"
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Address PoliCheck issues",
"packageName": "react-native-windows",
"email": "jthysell@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Address PoliCheck issues",
"packageName": "react-native-windows-init",
"email": "jthysell@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

// TS 3.8 type-only imports lead to parse errors with versions of prettier
// earlier than 2.0, which React Native uses as of 0.63. Isolate the type
// import to just this file and blacklist the file to get around it. Note that
// import to just this file and blocklist the file to get around it. Note that
// we cannot import the real module because it may be in a different location
// for older versions of RNW.
import type {generateWindows, GenerateOptions} from '@react-native-windows/cli';
Expand Down
8 changes: 4 additions & 4 deletions vnext/CHANGELOG.json
Original file line number Diff line number Diff line change
Expand Up @@ -11549,7 +11549,7 @@
"package": "react-native-windows"
},
{
"comment": "Code Cleanup: Some low haning fruit lint fixes",
"comment": "Code Cleanup: Some easy lint fixes",
"author": "dannyvv@microsoft.com",
"commit": "7361c32af8cce87267be33c1d1cfc9fb6368274c",
"package": "react-native-windows"
Expand Down Expand Up @@ -11620,7 +11620,7 @@
"comments": {
"prerelease": [
{
"comment": "blacklist all ProjectImports.zip",
"comment": "blocklist all ProjectImports.zip",
"author": "asklar@microsoft.com",
"commit": "e2d328a545a2520f760dbd0c782e2da20f0eb355",
"package": "react-native-windows"
Expand Down Expand Up @@ -12796,7 +12796,7 @@
"package": "react-native-windows"
},
{
"comment": "Add msbuild.ProjectImports.zip to default blacklist to avoid metro error on run-windows",
"comment": "Add msbuild.ProjectImports.zip to default blocklist to avoid metro error on run-windows",
"author": "acoates@microsoft.com",
"commit": "83f6067f50abd97e6bb2f59d41afe9dad04e26d0",
"package": "react-native-windows"
Expand Down Expand Up @@ -18365,4 +18365,4 @@
}
}
]
}
}
6 changes: 3 additions & 3 deletions vnext/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4366,7 +4366,7 @@ Wed, 03 Jun 2020 00:05:25 GMT

- Deprecate acceptsKeyboardFocus (kaigu@microsoft.com)
- Remove react-native-community/cli dependency from template (acoates@microsoft.com)
- Code Cleanup: Some low haning fruit lint fixes (dannyvv@microsoft.com)
- Code Cleanup: Some easy lint fixes (dannyvv@microsoft.com)
- Allow paremeterization of buildLogDirectory for msbuild task when using run-windows (dannyvv@microsoft.com)
- Minor autolinking template update prep (jthysell@microsoft.com)

Expand All @@ -4392,7 +4392,7 @@ Sun, 31 May 2020 00:05:21 GMT

### Changes

- blacklist all ProjectImports.zip (asklar@microsoft.com)
- blocklist all ProjectImports.zip (asklar@microsoft.com)
- docs CI (asklar@microsoft.com)

## 0.0.0-master.79
Expand Down Expand Up @@ -4768,7 +4768,7 @@ Fri, 17 Apr 2020 00:04:27 GMT
### Changes

- run_wdio.js will run the tests but also set the exit code to zero/non-zero on success/failure respectively. This is important to break the CI/PR build on test failures, which we weren't doing until now. (asklar@winse.microsoft.com)
- Add msbuild.ProjectImports.zip to default blacklist to avoid metro error on run-windows (acoates@microsoft.com)
- Add msbuild.ProjectImports.zip to default blocklist to avoid metro error on run-windows (acoates@microsoft.com)
- implement accessibilityState (kmelmon@microsoft.com)

## 0.0.0-master.41
Expand Down
2 changes: 1 addition & 1 deletion vnext/Desktop/ABI/NativeLogging.idl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Questions:
// - Should we approximate the existing API as closely as possible (a) or should we re-design the
// API using IDL/WinRT/COM idioms (b)? Examples for (b) are delegates, property accessors and
// events. (a) might help developers to orient themselves quicker, simplify migration to the new
// events. (a) might help developers to get started quicker, simplify migration to the new
// ABI and reduce refactoring-related defects. (b) should get us closer to the "final" API and
// might foster programming paradigms that reduce defects.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class UIManagerModule : public std::enable_shared_from_this<UIManagerModule>, pu
writer.WriteObjectEnd();

auto val = TakeJSValue(writer);
return std::move(val.AsObject().Copy()); // Lame why do we need to copy?
return std::move(val.AsObject().Copy()); // Why do we need to copy?
}

void ConstantsViaConstantsProvider(winrt::Microsoft::ReactNative::ReactConstantProvider &constants) noexcept {
Expand Down
12 changes: 6 additions & 6 deletions vnext/Shared/Networking/OriginPolicyHttpFilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,12 @@ bool OriginPolicyHttpFilter::ConstWcharComparer::operator()(const wchar_t *a, co
}

/*static*/ bool OriginPolicyHttpFilter::IsSimpleCorsRequest(HttpRequestMessage const &request) noexcept {
// Ensure header is in Simple CORS white list
// Ensure header is in Simple CORS allowlist
for (const auto &header : request.Headers()) {
if (s_simpleCorsRequestHeaderNames.find(header.Key().c_str()) == s_simpleCorsRequestHeaderNames.cend())
return false;

// Ensure Content-Type value is in Simple CORS white list, if present
// Ensure Content-Type value is in Simple CORS allowlist, if present
if (boost::iequals(header.Key(), L"Content-Type")) {
if (s_simpleCorsContentTypeValues.find(header.Value().c_str()) != s_simpleCorsContentTypeValues.cend())
return false;
Expand All @@ -138,20 +138,20 @@ bool OriginPolicyHttpFilter::ConstWcharComparer::operator()(const wchar_t *a, co
// WinRT separates request headers from request content headers
if (auto content = request.Content()) {
for (const auto &header : content.Headers()) {
// WinRT automatically appends non-whitelisted header Content-Length when Content-Type is set. Skip it.
// WinRT automatically appends non-allowlisted header Content-Length when Content-Type is set. Skip it.
if (s_simpleCorsRequestHeaderNames.find(header.Key().c_str()) == s_simpleCorsRequestHeaderNames.cend() &&
!boost::iequals(header.Key(), "Content-Length"))
return false;

// Ensure Content-Type value is in Simple CORS white list, if present
// Ensure Content-Type value is in Simple CORS allowlist, if present
if (boost::iequals(header.Key(), L"Content-Type")) {
if (s_simpleCorsContentTypeValues.find(header.Value().c_str()) == s_simpleCorsContentTypeValues.cend())
return false;
}
}
}

// Ensure method is in Simple CORS white list
// Ensure method is in Simple CORS allowlist
return s_simpleCorsMethods.find(request.Method().ToString().c_str()) != s_simpleCorsMethods.cend();
}

Expand Down Expand Up @@ -600,7 +600,7 @@ void OriginPolicyHttpFilter::ValidateResponse(HttpResponseMessage const &respons
}

if (originPolicy == OriginPolicy::SimpleCrossOriginResourceSharing) {
// Filter out response headers that are not in the Simple CORS whitelist
// Filter out response headers that are not in the Simple CORS allowlist
std::queue<hstring> nonSimpleNames;
for (const auto &header : response.Headers().GetView()) {
if (s_simpleCorsResponseHeaderNames.find(header.Key().c_str()) == s_simpleCorsResponseHeaderNames.cend())
Expand Down

0 comments on commit 0512b19

Please sign in to comment.