Skip to content

Add /tx-digest-hash endpoint to fetch last x tx digests#88

Open
tanuj-shardeum wants to merge 1 commit into
devfrom
BLUE-281
Open

Add /tx-digest-hash endpoint to fetch last x tx digests#88
tanuj-shardeum wants to merge 1 commit into
devfrom
BLUE-281

Conversation

@tanuj-shardeum

@tanuj-shardeum tanuj-shardeum commented Sep 30, 2024

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

github-actions Bot commented Sep 30, 2024

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Key issues to review

Type Validation
The type validation for the 'count' parameter uses a string 's' which might not be correct. It should be validated against a numeric type since 'count' is expected to be a number.

Error Handling
The error message "Invalid count" is vague. It should specify that the count must be a positive number.

github-advanced-security[bot]
github-advanced-security AI found potential problems Sep 30, 2024
View reviewed changes
Comment thread src/txDigester/api.ts
count = config.REQUEST_LIMIT.MAX_DIGESTS_PER_REQUEST
}

console.log(`Fetching latest ${count} tx digests`)

Check warning

Code scanning / CodeQL

Log injection

Log entry depends on a [user-provided value](1).

Copilot Autofix

AI almost 2 years ago

To fix the log injection issue, we need to sanitize the count parameter before using it in the log statement. Specifically, we should ensure that the count parameter does not contain any characters that could be interpreted as new lines or other control characters. This can be achieved by converting the count to a string and replacing any newline characters with an empty string.

Suggested changeset 1
src/txDigester/api.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/txDigester/api.ts b/src/txDigester/api.ts
--- a/src/txDigester/api.ts
+++ b/src/txDigester/api.ts
@@ -56,4 +56,5 @@
     }
-    
-    console.log(`Fetching latest ${count} tx digests`)
+    // Sanitize the count parameter to prevent log injection
+    const sanitizedCount = count.toString().replace(/\n|\r/g, "")
+    console.log(`Fetching latest ${sanitizedCount} tx digests`)
     const txDigests = await getLatestTxDigests(count)
EOF
@@ -56,4 +56,5 @@
}

console.log(`Fetching latest ${count} tx digests`)
// Sanitize the count parameter to prevent log injection
const sanitizedCount = count.toString().replace(/\n|\r/g, "")
console.log(`Fetching latest ${sanitizedCount} tx digests`)
const txDigests = await getLatestTxDigests(count)
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jairajdev jairajdev Awaiting requested review from jairajdev

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Review effort [1-5]: 3

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tanuj-shardeum @github-advanced-security