Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check UI hashes Part 2 #48

Merged
merged 5 commits into from
Mar 21, 2024
Merged

check UI hashes Part 2 #48

merged 5 commits into from
Mar 21, 2024

Conversation

matthme
Copy link
Collaborator

@matthme matthme commented Mar 18, 2024

  • computes the hashes from the bytes in memory instead of writing them to a zip file in the temp directory
  • adds better error handling when checking hashes before serving asset to happ window
  • adds test for IntegrityChecker

@matthme matthme requested review from ThetaSinner and mrruby March 18, 2024 15:55
iC.storeToSignedJSON(filePath, writeData);
// tamper with file
const readString = fs.readFileSync(filePath, 'utf-8');
fs.writeFileSync(filePath, readString + 'tampered');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I's be careful to maintain the structure of the file here. This could (and I think will) throw because the content is no longer valid JSON. It's the signature that's important here so it needs to be that error that gets thrown

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valid point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, should be addressed with the latest commit

Copy link
Member

@ThetaSinner ThetaSinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just one comment on the tampering test

Copy link
Member

@ThetaSinner ThetaSinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@matthme matthme merged commit abb4025 into develop Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants