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

Changes to contract CIDs (requires updating the chel command) #2494

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

corrideat
Copy link
Member

@corrideat corrideat requested a review from taoeffect January 5, 2025 20:37
@corrideat corrideat added this to the Final breaking changes milestone Jan 5, 2025
@corrideat
Copy link
Member Author

multiformats/multicodec#369 merged

@corrideat corrideat self-assigned this Jan 7, 2025
Copy link

socket-security bot commented Jan 18, 2025

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@chelonia/cli@3.0.0 🔁 npm/@chelonia/cli@2.2.3 Transitive: environment, network +8 825 kB taoeffect

View full report↗︎

@corrideat corrideat force-pushed the 2115-investigate-potential-ways-of-preventing-malicious-contracts-from-being-loaded branch from 36d8709 to 942d1cb Compare January 18, 2025 18:33
@corrideat corrideat marked this pull request as ready for review January 18, 2025 18:39
Copy link

cypress bot commented Jan 18, 2025

group-income    Run #4111

Run Properties:  status check passed Passed #4111  •  git commit ddce5dc00e ℹ️: Merge d4ac16012288bf4d561a5d47ba5480cdc09fdf69 into 023ea135b73e8f943dfb7a025402...
Project group-income
Branch Review 2115-investigate-potential-ways-of-preventing-malicious-contracts-from-being-loaded
Run status status check passed Passed #4111
Run duration 11m 43s
Commit git commit ddce5dc00e ℹ️: Merge d4ac16012288bf4d561a5d47ba5480cdc09fdf69 into 023ea135b73e8f943dfb7a025402...
Committer Ricardo Iván Vieitez Parra
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 10
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 114
View all changes introduced in this branch ↗︎

@corrideat corrideat force-pushed the 2115-investigate-potential-ways-of-preventing-malicious-contracts-from-being-loaded branch from 7fdc3f7 to 6b02beb Compare February 15, 2025 23:26
@corrideat corrideat force-pushed the 2115-investigate-potential-ways-of-preventing-malicious-contracts-from-being-loaded branch from 6e6cd09 to daef9a9 Compare February 20, 2025 16:40
Comment on lines 852 to 856
export const CONTRACT_MANIFEST_REGEX: RegExp = /^zL7mM9d4Xb4T[123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz]{44}$/
export const CONTRACT_SOURCE_REGEX: RegExp = /^zLAeVmpcc88g[123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz]{44}$/
export const CONTRACT_DATA_REGEX: RegExp = /^zLDXeQ2AgfCu[123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz]{44}$/
export const FILE_MANIFEST_REGEX: RegExp = /^zLGQo2DimCH8{44}$/
export const FILE_CHUNK_REGEX: RegExp = /^zLKHweRGqjMM{44}$/
Copy link
Member

Choose a reason for hiding this comment

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

Why are the last two missing [123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz]?

Copy link
Member Author

Choose a reason for hiding this comment

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

Bad copy & paste.

Gruntfile.js Outdated
Comment on lines 267 to 269
// This banner makes contracts easy to detect and harmless in the event
// of accidental execution
js: 'for(;;)"use shelter";'
Copy link
Member

@taoeffect taoeffect Feb 25, 2025

Choose a reason for hiding this comment

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

Please update this comment to say that the reason this is being added is to prevent people from uploading malicious contracts to our servers and then loading them elsewhere (e.g. via XSS or whatever).

EDIT: however, this isn't enforced server side, so it seems there's no point to it.

IMO, unless it's enforced server-side, we shouldn't have this as it's unnecessary complexity that accomplishes nothing. And if we enforce it server-side, then we can't support other languages — something we want.

So please could you remove this banner and related code?

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.

Investigate: potential ways of preventing malicious contracts from being loaded
2 participants