Skip to content

Commit

Permalink
Linter checks: dom manipulation, network traffic, and direct storage …
Browse files Browse the repository at this point in the history
…access && bugfixes: unauthorized storage access (#11884)

* relevatehealthBidAdapter.js: bugfix for storage used without consent

* Update index.js

* Update index.js

* Update index.js

* Update index.js

* Update index.js

* Update index.js

* Update index.js

* Update index.js

* Update relevatehealthBidAdapter.js

* Update index.js

* Update index.js

* Update index.js

* Update index.js

* Update index.js

* Update .eslintrc.js

* Update index.js

* Update relevatehealthBidAdapter.js

* Update 33acrossAnalyticsAdapter.js

* Update index.js

* Update index.js

* Update 33acrossAnalyticsAdapter.js

* Update index.js

* Update ampliffyBidAdapter.js

* Update index.js

* Update invibesBidAdapter.js

* Update growthCodeAnalyticsAdapter.js

* Update fintezaAnalyticsAdapter.js

* Update growthCodeAnalyticsAdapter.js

* Update etargetBidAdapter.js

* Update dspxBidAdapter.js

* Update cwireBidAdapter.js

* Update cwireBidAdapter.js

* Update ampliffyBidAdapter.js

* Update etargetBidAdapter.js

* Update dspxBidAdapter.js

* Update fintezaAnalyticsAdapter.js

* Update ampliffyBidAdapter.js

* Update adlooxAnalyticsAdapter.js

* Update invibesBidAdapter.js

* Update fintezaAnalyticsAdapter.js

* Update dspxBidAdapter.js

* Update connectIdSystem.js

* Update automatadAnalyticsAdapter.js

* Update sonobiBidAdapter.js

* Update contxtfulRtdProvider.js

* Update sonobiBidAdapter.js

* Update contxtfulRtdProvider.js

* Update index.js

* Update cleanioRtdProvider.js

* Update connectIdSystem.js

* Update geoedgeRtdProvider.js

* Update growthCodeRtdProvider.js

* Update sirdataRtdProvider.js

* Update sirdataRtdProvider.js

* Update contxtfulRtdProvider_spec.js

* Update contxtfulRtdProvider_spec.js

* Update index.js

Fix duplication

* Update index.js

* refactor custom linter rules

* use TODO and comments instead of quietly removing rule-breaking code

* Add linter GH action

* Fix linter workflow name

* Run npm ci on linter check

* Filter out missing (new) files

* Do not fail on linter failure

* swap continue-on-error

* remove spurious condition

* Improve comment

* Fix links for duplication checker comments

* Filter out negative deltas

* Update linter warning comment

* Update .eslintrc.js

---------

Co-authored-by: Demetrio Girardi <[email protected]>
  • Loading branch information
patmmccann and dgirardi authored Jul 4, 2024
1 parent 0110b3c commit a4cc27c
Show file tree
Hide file tree
Showing 24 changed files with 266 additions and 70 deletions.
35 changes: 30 additions & 5 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,27 +84,52 @@ module.exports = {
files: key + '/**/*.js',
rules: {
'prebid/validate-imports': ['error', allowedModules[key]],
'prebid/no-innerText': ['error', allowedModules[key]],
'no-restricted-globals': [
'error',
{
name: 'require',
message: 'use import instead'
}
],
'prebid/no-global': [
'error',
...['localStorage', 'sessionStorage'].map(name => ({name, message: 'use storageManager instead'})),
{
name: 'XMLHttpRequest',
message: 'use ajax.js instead'
},
],
'prebid/no-member': [
'error',
{
name: 'cookie',
target: 'document',
message: 'use storageManager instead'
},
{
name: 'sendBeacon',
target: 'navigator',
message: 'use ajax.js instead'
},
...['outerText', 'innerText'].map(name => ({
name,
message: 'use .textContent instead'
}))
]
}
})).concat([{
// code in other packages (such as plugins/eslint) is not "seen" by babel and its parser will complain.
files: 'plugins/*/**/*.js',
parser: 'esprima'
},
{
}, {
files: '**BidAdapter.js',
rules: {
'no-restricted-imports': [
'error', {
patterns: ["**/src/events.js",
"**/src/adloader.js"]
patterns: [
'**/src/events.js',
'**/src/adloader.js'
]
}
]
}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/jscpd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ jobs:
const filteredReport = JSON.parse(fs.readFileSync('filtered-jscpd-report.json', 'utf8'));
let comment = "Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:\n\n";
function link(dup) {
return `https://github.com/${{ github.event.repository.full_name }}/blob/${{ github.event.pull_request.head.sha }}/${dup.name}#L${dup.start}-L${dup.end - 1}`
return `https://github.com/${{ github.event.repository.full_name }}/blob/${{ github.event.pull_request.head.sha }}/${dup.name}#L${dup.start + 1}-L${dup.end - 1}`
}
filteredReport.forEach(duplication => {
const firstFile = duplication.firstFile;
Expand Down
107 changes: 107 additions & 0 deletions .github/workflows/linter.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
name: Check for linter warnings / exceptions

on:
pull_request_target:
branches:
- master

jobs:
check-linter:
runs-on: ubuntu-latest

steps:
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: '20'

- name: Checkout code
uses: actions/checkout@v4
with:
fetch-depth: 0
ref: ${{ github.event.pull_request.base.sha }}

- name: Fetch base and target branches
run: |
git fetch origin +refs/heads/${{ github.event.pull_request.base.ref }}:refs/remotes/origin/${{ github.event.pull_request.base.ref }}
git fetch origin +refs/pull/${{ github.event.pull_request.number }}/merge:refs/remotes/pull/${{ github.event.pull_request.number }}/merge
- name: Install dependencies
run: npm ci

- name: Get the diff
run: git diff --name-only origin/${{ github.event.pull_request.base.ref }}...refs/remotes/pull/${{ github.event.pull_request.number }}/merge > __changed_files.txt

- name: Run linter on base branch
run: npx eslint --no-inline-config --format json $(cat __changed_files.txt | xargs stat --printf '%n\n' 2> /dev/null) > __base.json || true

- name: Check out PR
run: git checkout ${{ github.event.pull_request.head.sha }}

- name: Run linter on PR
run: npx eslint --no-inline-config --format json $(cat __changed_files.txt | xargs stat --printf '%n\n' 2> /dev/null) > __pr.json || true

- name: Compare them and post comment if necessary
uses: actions/github-script@v7
with:
script: |
const fs = require('fs');
const path = require('path');
const process = require('process');
function parse(fn) {
return JSON.parse(fs.readFileSync(fn)).reduce((memo, data) => {
const file = path.relative(process.cwd(), data.filePath);
if (!memo.hasOwnProperty(file)) { memo[file] = { errors: 0, warnings: 0} }
data.messages.forEach(({severity}) => {
memo[file][severity > 1 ? 'errors' : 'warnings']++;
});
return memo;
}, {})
}
function mkDiff(old, new_) {
const files = Object.fromEntries(
Object.entries(new_)
.map(([file, {errors, warnings}]) => {
const {errors: oldErrors, warnings: oldWarnings} = old[file] || {};
return [file, {errors: Math.max(0, errors - (oldErrors ?? 0)), warnings: Math.max(0, warnings - (oldWarnings ?? 0))}]
})
.filter(([_, {errors, warnings}]) => errors > 0 || warnings > 0)
)
return Object.values(files).reduce((memo, {warnings, errors}) => {
memo.errors += errors;
memo.warnings += warnings;
return memo;
}, {errors: 0, warnings: 0, files})
}
function mkComment({errors, warnings, files}) {
function pl(noun, number) {
return noun + (number === 1 ? '' : 's')
}
if (errors === 0 && warnings === 0) return;
const summary = [];
if (errors) summary.push(`**${errors}** linter ${pl('error', errors)}`)
if (warnings) summary.push(`**${warnings}** linter ${pl('warning', warnings)}`)
let cm = `Tread carefully! This PR adds ${summary.join(' and ')} (possibly disabled through directives):\n\n`;
Object.entries(files).forEach(([file, {errors, warnings}]) => {
const summary = [];
if (errors) summary.push(`+${errors} ${pl('error', errors)}`);
if (warnings) summary.push(`+${warnings} ${pl('warning', warnings)}`)
cm += ` * \`${file}\` (${summary.join(', ')})\n`
})
return cm;
}
const [base, pr] = ['__base.json', '__pr.json'].map(parse);
const comment = mkComment(mkDiff(base, pr));
if (comment) {
github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
body: comment
});
}
2 changes: 2 additions & 0 deletions modules/33acrossAnalyticsAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,8 @@ function setCachedBidStatus(auctionId, bidId, status) {
* @param {string} endpoint URL
*/
function sendReport(report, endpoint) {
// TODO FIX THIS RULES VIOLATION
// eslint-disable-next-line
if (navigator.sendBeacon(endpoint, JSON.stringify(report))) {
log.info(`Analytics report sent to ${endpoint}`, report);

Expand Down
1 change: 1 addition & 0 deletions modules/adlooxAnalyticsAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ analyticsAdapter[`handle_${EVENTS.AUCTION_END}`] = function(auctionDetails) {
link.setAttribute('href', `${uri.protocol}://${uri.host}${uri.pathname}`);
link.setAttribute('rel', 'preload');
link.setAttribute('as', 'script');
// TODO fix rules violation
insertElement(link);
}

Expand Down
2 changes: 2 additions & 0 deletions modules/automatadAnalyticsAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ var isLoggingEnabled; var queuePointer = 0; var retryCount = 0; var timer = null

const prettyLog = (level, text, isGroup = false, cb = () => {}) => {
if (self.isLoggingEnabled === undefined) {
// TODO FIX THIS RULES VIOLATION
// eslint-disable-next-line prebid/no-global
if (window.localStorage.getItem('__aggLoggingEnabled')) {
self.isLoggingEnabled = true
} else {
Expand Down
1 change: 1 addition & 0 deletions modules/cleanioRtdProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ let preloadStatus = 0;
* @param {string} scriptURL The script URL to preload
*/
function pageInitStepPreloadScript(scriptURL) {
// TODO: this bypasses adLoader
const linkElement = document.createElement('link');
linkElement.rel = 'preload';
linkElement.as = 'script';
Expand Down
4 changes: 3 additions & 1 deletion modules/connectIdSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,13 @@ export const connectIdSubmodule = {

/**
* Utility function that returns a boolean flag indicating if the user
* has opeted out via the Yahoo easy-opt-out mechanism.
* has opted out via the Yahoo easy-opt-out mechanism.
* @returns {Boolean}
*/
userHasOptedOut() {
try {
// TODO FIX THIS RULES VIOLATION
// eslint-disable-next-line
return localStorage.getItem(OVERRIDE_OPT_OUT_KEY) === '1';
} catch {
return false;
Expand Down
5 changes: 4 additions & 1 deletion modules/contxtfulRtdProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ function getRxEngineReceptivity(requester) {
}

function loadSessionReceptivity(requester) {
// TODO: commented out because of rule violations
/*
let sessionStorageValue = sessionStorage.getItem(requester);
if (!sessionStorageValue) {
return null;
Expand All @@ -54,7 +56,8 @@ function loadSessionReceptivity(requester) {
} catch {
return null;
}
};
*/
}

/**
* Prepare a receptivity batch
Expand Down
4 changes: 4 additions & 0 deletions modules/cwireBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ export const spec = {
bid: bid
}
}
// TODO FIX THIS RULES VIOLATION
// eslint-disable-next-line prebid/no-member
navigator.sendBeacon(EVENT_ENDPOINT, JSON.stringify(event))
},

Expand All @@ -236,6 +238,8 @@ export const spec = {
bidderRequest: bidderRequest
}
}
// TODO FIX THIS RULES VIOLATION
// eslint-disable-next-line prebid/no-member
navigator.sendBeacon(EVENT_ENDPOINT, JSON.stringify(event))
},

Expand Down
3 changes: 3 additions & 0 deletions modules/debugging/debugging.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export function disableDebugging({hook, logger}) {
}
}

// eslint-disable-next-line prebid/no-global
function saveDebuggingConfig(debugConfig, {sessionStorage = window.sessionStorage, DEBUG_KEY} = {}) {
if (!debugConfig.enabled) {
try {
Expand All @@ -49,6 +50,7 @@ function saveDebuggingConfig(debugConfig, {sessionStorage = window.sessionStorag
}
}

// eslint-disable-next-line prebid/no-global
export function getConfig(debugging, {getStorage = () => window.sessionStorage, DEBUG_KEY, config, hook, logger} = {}) {
if (debugging == null) return;
let sessionStorage;
Expand All @@ -70,6 +72,7 @@ export function getConfig(debugging, {getStorage = () => window.sessionStorage,
export function sessionLoader({DEBUG_KEY, storage, config, hook, logger}) {
let overrides;
try {
// eslint-disable-next-line prebid/no-global
storage = storage || window.sessionStorage;
overrides = JSON.parse(storage.getItem(DEBUG_KEY));
} catch (e) {
Expand Down
2 changes: 2 additions & 0 deletions modules/dgkeywordRtdProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ export function getProfileApiUrl(customeUrl, enableReadFpid) {

export function readFpidFromLocalStrage() {
try {
// TODO: use storageManager
// eslint-disable-next-line prebid/no-global
const fpid = window.localStorage.getItem('ope_fpid');
if (fpid) {
return fpid;
Expand Down
13 changes: 9 additions & 4 deletions modules/fintezaAnalyticsAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ function initFirstVisit() {
let cookies;

try {
cookies = parseCookies(document.cookie);
// TODO: commented out because of rule violations
cookies = {} // parseCookies(document.cookie);
} catch (a) {
cookies = {};
}
Expand All @@ -91,7 +92,8 @@ function initFirstVisit() {

return visitDate;
}

// TODO: commented out because of rule violations
/*
function trim(string) {
if (string.trim) {
return string.trim();
Expand Down Expand Up @@ -130,6 +132,7 @@ function parseCookies(cookie) {
return values;
}
*/

function getRandAsStr(digits) {
let str = '';
Expand Down Expand Up @@ -172,7 +175,8 @@ function initSession() {
let isNew = false;

try {
cookies = parseCookies(document.cookie);
// TODO: commented out because of rule violations
cookies = {} // parseCookies(document.cookie);
} catch (a) {
cookies = {};
}
Expand Down Expand Up @@ -263,7 +267,8 @@ function getTrackRequestLastTime() {
);
}

cookie = parseCookies(document.cookie);
// TODO: commented out because of rule violations
cookie = {} // parseCookies(document.cookie);
cookie = cookie[ TRACK_TIME_KEY ];
if (cookie) {
return parseInt(cookie, 10);
Expand Down
2 changes: 1 addition & 1 deletion modules/growthCodeAnalyticsAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ function logToServer() {
if (pid === DEFAULT_PID) return;
if (eventQueue.length >= 1) {
// Get the correct GCID
let gcid = localStorage.getItem('gcid')
let gcid = storage.getDataFromLocalStorage('gcid');

let data = {
session: sessionId,
Expand Down
2 changes: 1 addition & 1 deletion modules/growthCodeRtdProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function callServer(configParams, items, expiresAt, userConsent) {
storage.removeDataFromLocalStorage(RTD_EXPIRE_KEY, null)
}
if ((items === null) && (isNaN(expiresAt))) {
let gcid = localStorage.getItem('gcid')
let gcid = storage.getDataFromLocalStorage('gcid')

let url = configParams.url ? configParams.url : ENDPOINT_URL;
url = tryAppendQueryString(url, 'pid', configParams.pid);
Expand Down
3 changes: 2 additions & 1 deletion modules/relevatehealthBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ function buildUser(bid) {
if (bid && bid.params) {
return {
id: bid.params.user_id && typeof bid.params.user_id == 'string' ? bid.params.user_id : '',
buyeruid: localStorage.getItem('adx_profile_guid') ? localStorage.getItem('adx_profile_guid') : '',
// TODO: commented out because of rule violations
buyeruid: '', // localStorage.getItem('adx_profile_guid') ? localStorage.getItem('adx_profile_guid') : '',
keywords: bid.params.keywords && typeof bid.params.keywords == 'string' ? bid.params.keywords : '',
customdata: bid.params.customdata && typeof bid.params.customdata == 'string' ? bid.params.customdata : ''
};
Expand Down
2 changes: 2 additions & 0 deletions modules/sirdataRtdProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ export function postContentForSemanticAnalysis(postContentToken, actualUrl) {

// Use the Beacon API if supported to send the payload
if ('sendBeacon' in navigator) {
// TODO FIX RULES VIOLATION
// eslint-disable-next-line prebid/no-member
navigator.sendBeacon(url, payload);
} else {
// Fallback to using AJAX if Beacon API is not supported
Expand Down
Loading

0 comments on commit a4cc27c

Please sign in to comment.