Add har recorder #181

thypon · 2024-12-16T19:32:13Z

Continues #180

github-actions · 2024-12-16T19:35:26Z

The following commits were not verified:
a39a942 (unsigned)

FHantke · 2024-12-16T20:00:04Z

@pes10k Can confirm, branch still works as expected.

FHantke · 2024-12-17T13:20:31Z

@pes10k I don't have the access rights to push here. Added tests to my fork https://github.com/FHantke/pagegraph-crawl/tree/add-har-recorder

github-actions · 2024-12-17T17:35:29Z

[puLL-Merge] - brave/pagegraph-crawl@181

Description

This PR introduces several significant changes to the pagegraph-crawl project, including the addition of HAR (HTTP Archive) generation functionality, improvements to the testing process, and updates to dependencies and documentation.

Changes

Changes

.github/workflows/pr.yml:
- Removed manual sed command for configuring the test binary path
- Added environment variables for debugging and specifying the test binary path
.gitignore:
- Added test/debug_output/ and test/output/ to ignore list
README.md:
- Updated node version requirement
- Minor text improvements
built/brave/crawl.js:
- Added HAR generation functionality
- Implemented prepareHARGenerator function to set up HAR recording
- Modified doCrawl function to include HAR generation
built/brave/files.js:
- Added writeHAR function
- Replaced fs-extra dependency with native node:fs/promises
built/brave/puppeteer.js:
- Updated profile path handling
- Removed commented-out stealth mode code
built/brave/validate.js:
- Added storeHar and storeHarBody options
- Replaced hasbin with which for binary detection
built/run.js:
- Added command-line arguments for HAR generation
changelog.md:
- Updated with new features and changes for version 1.1.3
eslint.config.js:
- Added rule to ignore camelcase for properties
package.json:
- Updated version to 1.1.3
- Added chrome-har and which dependencies
- Removed fs-extra, hasbin, and chai dependencies
- Updated scripts and dev dependencies
src/ directory:
- Updated TypeScript source files to reflect changes in built JavaScript files
test/:
- Removed config.js
- Added new test pages and resources
- Significantly refactored test.js:
  - Replaced chai with native assert
  - Implemented HAR validation
  - Added tests for HAR generation
- Updated test_server.js (changes not shown in the diff)

sequenceDiagram
    participant User
    participant CLI
    participant Puppeteer
    participant Browser
    participant HARGenerator
    participant FileSystem

    User->>CLI: Run crawl command
    CLI->>Puppeteer: Launch browser
    Puppeteer->>Browser: Create new page
    CLI->>HARGenerator: Prepare HAR recording
    Browser->>HARGenerator: Send network events
    Browser->>FileSystem: Generate PageGraph
    HARGenerator->>FileSystem: Generate HAR file
    FileSystem->>User: Save output files

Possible Issues

The removal of the fs-extra dependency might impact file operations that relied on its extended functionality.
The switch from hasbin to which for binary detection could potentially behave differently in edge cases.

Security Hotspots

The new HAR generation feature stores response bodies, which could potentially include sensitive information if not handled carefully.

socket-security · 2024-12-17T17:35:29Z

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert	Package	Note	Source	CI
Deprecated	npm/rimraf@3.0.2	Reason: Rimraf versions prior to v4 are no longer supported	`package-lock.json` `package.json`	⚠︎
Deprecated	npm/@humanwhocodes/object-schema@2.0.3	Reason: Use @eslint/object-schema instead	`package-lock.json` `package.json`	⚠︎
Deprecated	npm/@humanwhocodes/config-array@0.13.0	Reason: Use @eslint/config-array instead	`package-lock.json` `package.json`	⚠︎
Deprecated	npm/eslint@8.57.1	Reason: This version is no longer supported. Please see https://eslint.org/version-support for other options.	`package-lock.json` `package.json`	⚠︎

View full report↗︎

Next steps

What is a deprecated package?

The maintainer of the package marked it as deprecated. This could indicate that a single version should not be used, or that the package is no longer maintained and any new vulnerabilities will not be fixed.

Research the state of the package and determine if there are non-deprecated versions that can be used, or if it should be replaced with a new, supported solution.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/foo@1.0.0 or ignore all packages with @SocketSecurity ignore-all

@SocketSecurity ignore npm/rimraf@3.0.2
@SocketSecurity ignore npm/@humanwhocodes/object-schema@2.0.3
@SocketSecurity ignore npm/@humanwhocodes/config-array@0.13.0
@SocketSecurity ignore npm/eslint@8.57.1

github-actions · 2024-12-17T17:36:22Z

test/pages/resources/document.svg

@@ -0,0 +1,4 @@
+<svg height="100" width="100" xmlns="http://www.w3.org/2000/svg">


_{reported by reviewdog 🐶}
[safesvg] element svg: validity error : Element svg content does not follow the DTD, expecting (desc | title | metadata | animate | set | animateMotion | animateColor | animateTransform | svg | g | defs | symbol | use | switch | image | style | path | rect | circle | line | ellipse | polyline | polygon | text | altGlyphDef | marker | color-profile | linearGradient | radialGradient | pattern | clipPath | mask | filter | cursor | a | view | script | font | font-face | foreignObject)*, got (circle CDATA)

Run SVGO on your assets

Cc @thypon @kdenhartog

diracdeltas · 2024-12-17T17:59:35Z

this repo has some deprecated dependencies that should be addressed (see the socket comment above)

pes10k · 2024-12-17T18:02:00Z

@diracdeltas these are just dev dependencies, and all pulled in from standardjs. Since they're just dev dependencies i was hoping they could be ignored.

If not, is there an alternative security recommends other than standardjs? IIRC it was either suggested or required for all code for a while. I can search for something kept more up to date, but if any suggestions greatly appreciated

diracdeltas · 2024-12-17T18:17:20Z

@diracdeltas these are just dev dependencies, and all pulled in from standardjs. Since they're just dev dependencies i was hoping they could be ignored.

If not, is there an alternative security recommends other than standardjs? IIRC it was either suggested or required for all code for a while. I can search for something kept more up to date, but if any suggestions greatly appreciated

we do use standard in basically all js repos but i haven't seen these issues. they might be false positives then.

diracdeltas · 2024-12-17T18:17:55Z

package.json

    "eslint": "9.16.0",
-    "express": "4.21.1",
+    "express": "^4.21.2",
+    "standard": "^17.1.2",


these updated deps should be pinned to exact versions instead of using ^

pes10k · 2024-12-17T18:24:31Z

@diracdeltas these are just dev dependencies, and all pulled in from standardjs. Since they're just dev dependencies i was hoping they could be ignored.

If not, is there an alternative security recommends other than standardjs? IIRC it was either suggested or required for all code for a while. I can search for something kept more up to date, but if any suggestions greatly appreciated

we do use standard in basically all js repos but i haven't seen these issues. they might be false positives then.

Hmm, odd, I don't think they're false positives. The current version of standard really does depend versions of eslint socket.io is complaining about (see https://github.com/standard/standard/blob/master/package.json#L18 for example, which is even older than the [8.57.1] version i've pulled in).

Are those other projects configured differently? Would really like to not need to remove standard here (its very handy) if possible

…endency

pes10k

Looks good. Discussing what to do with socket.io warnings in other channels

Florian Hantke and others added 2 commits December 8, 2024 17:52

enable har recording

a39a942

removing fs-extras dep, linter and formatting fixes, test runner cleanup

5a6117e

thypon requested a review from pes10k as a code owner December 16, 2024 19:32

pes10k added 2 commits December 16, 2024 13:40

removing fs-extras dep, linter and formatting fixes, test runner cleanup

e98d95f

add debugging to tests

913929f

thypon mentioned this pull request Dec 16, 2024

Enable HAR Recordings #180

Closed

pes10k force-pushed the add-har-recorder branch 2 times, most recently from fc59bfd to 2e89553 Compare December 16, 2024 20:31

fix how we guess binaries in PATH

e8ae1ad

pes10k force-pushed the add-har-recorder branch from 2e89553 to e8ae1ad Compare December 16, 2024 20:43

add HAR tests

ed6f4b2

github-actions bot added the puLL-Merge label Dec 17, 2024

github-actions bot reviewed Dec 17, 2024

View reviewed changes

github-actions bot assigned kdenhartog and thypon Dec 17, 2024

pes10k force-pushed the add-har-recorder branch 2 times, most recently from b32d9d3 to ae8b73a Compare December 17, 2024 17:51

diracdeltas reviewed Dec 17, 2024

View reviewed changes

pes10k force-pushed the add-har-recorder branch 3 times, most recently from 5605d9c to ce01a2a Compare December 17, 2024 18:38

add tests from @FHantke, add linting to test scripts, remove chai dep…

ea6c82f

…endency

pes10k force-pushed the add-har-recorder branch from ce01a2a to ea6c82f Compare December 17, 2024 18:51

pes10k approved these changes Dec 17, 2024

View reviewed changes

pes10k merged commit 4594616 into main Dec 17, 2024
5 checks passed

pes10k deleted the add-har-recorder branch December 17, 2024 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add har recorder #181

Add har recorder #181

thypon commented Dec 16, 2024

github-actions bot commented Dec 16, 2024

FHantke commented Dec 16, 2024

FHantke commented Dec 17, 2024

github-actions bot commented Dec 17, 2024

Changes

socket-security bot commented Dec 17, 2024 •

edited

Loading

github-actions bot Dec 17, 2024

diracdeltas commented Dec 17, 2024

pes10k commented Dec 17, 2024

diracdeltas commented Dec 17, 2024

diracdeltas Dec 17, 2024

pes10k commented Dec 17, 2024 •

edited

Loading

pes10k left a comment

		@@ -0,0 +1,4 @@
		<svg height="100" width="100" xmlns="http://www.w3.org/2000/svg">

Add har recorder #181

Add har recorder #181

Conversation

thypon commented Dec 16, 2024

github-actions bot commented Dec 16, 2024

FHantke commented Dec 16, 2024

FHantke commented Dec 17, 2024

github-actions bot commented Dec 17, 2024

Description

Changes

Possible Issues

Security Hotspots

socket-security bot commented Dec 17, 2024 • edited Loading

Next steps

github-actions bot Dec 17, 2024

Choose a reason for hiding this comment

diracdeltas commented Dec 17, 2024

pes10k commented Dec 17, 2024

diracdeltas commented Dec 17, 2024

diracdeltas Dec 17, 2024

Choose a reason for hiding this comment

pes10k commented Dec 17, 2024 • edited Loading

pes10k left a comment

Choose a reason for hiding this comment

socket-security bot commented Dec 17, 2024 •

edited

Loading

pes10k commented Dec 17, 2024 •

edited

Loading