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

feat: replay attack prevention using watermarks #854

Merged
merged 18 commits into from
Mar 13, 2024

Conversation

krpeacock
Copy link
Contributor

@krpeacock krpeacock commented Feb 21, 2024

Description

Several reports have been received about an issue affecting how immediately a response from an update call can be used. One example is the following scenario:

an update call is made to create a new resource (e.g. a social media post)

the application navigates to a page that displays the resource (e.g. the post itself)

the application requests the resource by executing a query call and using the resource id

the query hits a replica whos state has not yet been updated

a 404 is returned

the problem is, the resource does exist, it just so happens that 1/3 of replicas may not necessarily be up to date at the same moment the other 2/3’s are

Fixes # SDK-1332

How Has This Been Tested?

New unit and end to end tests, introducing a replay attack test in e2e/node/basic/watermark.test.ts

Checklist:

  • My changes follow the guidelines in CONTRIBUTING.md.
  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

@krpeacock krpeacock requested a review from a team as a code owner February 21, 2024 23:38
@krpeacock krpeacock changed the base branch from main to kyle/package-updates February 22, 2024 00:28
@krpeacock krpeacock changed the base branch from kyle/package-updates to main February 22, 2024 00:29
Copy link
Contributor

github-actions bot commented Feb 23, 2024

size-limit report 📦

Path Size
@dfinity/agent 101.81 KB (+0.7% 🔺)
@dfinity/candid 13.61 KB (0%)
@dfinity/principal 5.04 KB (0%)
@dfinity/auth-client 92.57 KB (+0.8% 🔺)
@dfinity/assets 93.43 KB (+0.61% 🔺)
@dfinity/identity 89.96 KB (+0.91% 🔺)
@dfinity/identity-secp256k1 281.19 KB (+0.31% 🔺)

Copy link
Member

@chmllr chmllr left a comment

Choose a reason for hiding this comment

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

Thanks so much Kyle! Please announce the release with this fix on Taggr, I'll give a ⭐️! 😅

packages/agent/src/agent/http/index.ts Outdated Show resolved Hide resolved
value: jest.fn(),
Object.defineProperty(global, 'console', {
writable: true,
value: { ...global.console, log: jest.fn(), warn: jest.fn(), error: jest.fn() },
Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean that no logs will be printed during test execution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's to clean up the console by default when running the tests. However, the logs aren't lost and you can access them as needed

@krpeacock krpeacock merged commit a38cb18 into main Mar 13, 2024
26 checks passed
@krpeacock krpeacock deleted the kai/SDK-1332-prevent-outdated-queries branch March 13, 2024 17:05
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.

None yet

3 participants