Skip to content

fix: enhance repository management with validation and pagination improvements #118

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 1, 2025
Merged

fix: enhance repository management with validation and pagination improvements #118

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 28 additions & 13 deletions contracts/RepositoryRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ contract RepositoryRegistry is Ownable, ReentrancyGuard {
require(repo.maintainer == msg.sender, "Not owner");
require(repo.isActive, "Inactive");

require(bytes(newDescription).length > 0, "Description cannot be empty");
repo.description = newDescription;
emit RepositoryUpdated(id, msg.sender);
}
Expand All @@ -81,8 +82,13 @@ contract RepositoryRegistry is Ownable, ReentrancyGuard {

// Check for duplicate repository name
for (uint256 i = 1; i <= repoCounter; i++) {
if (repositories[i].maintainer != address(0) && keccak256(abi.encodePacked(repositories[i].name)) == keccak256(abi.encodePacked(name))) {
revert("Repository name already exists");
if (repositories[i].maintainer != address(0)) {
if (keccak256(abi.encodePacked(repositories[i].name)) == keccak256(abi.encodePacked(name))) {
revert("Repository name already exists");
}
if (keccak256(abi.encodePacked(repositories[i].githubUrl)) == keccak256(abi.encodePacked(url))) {
revert("Repository URL already exists");
}
}
}

Expand Down Expand Up @@ -161,31 +167,40 @@ contract RepositoryRegistry is Ownable, ReentrancyGuard {
}

/**
* @dev Get all active repositories
* @dev Get active repositories with pagination
* @param offset The starting index (0-based)
* @param limit The maximum number of repositories to return
* @return Array of Repository structs that are currently active
*/
function getActiveRepositories() external view returns (Repository[] memory) {
// First pass: count active repositories
function getActiveRepositories(uint256 offset, uint256 limit) external view returns (Repository[] memory) {
// Gather all active repositories
uint256 activeCount = 0;
for (uint256 i = 1; i <= repoCounter; i++) {
if (repositories[i].isActive && repositories[i].maintainer != address(0)) {
activeCount++;
}
}

// Create array with exact size needed
Repository[] memory activeRepos = new Repository[](activeCount);

// Second pass: populate the array
Repository[] memory tempRepos = new Repository[](activeCount);
uint256 index = 0;
for (uint256 i = 1; i <= repoCounter; i++) {
if (repositories[i].isActive && repositories[i].maintainer != address(0)) {
activeRepos[index] = repositories[i];
tempRepos[index] = repositories[i];
index++;
}
}

return activeRepos;
// Pagination
if (offset >= activeCount) {
return new Repository[](0);
}
uint256 end = offset + limit;
if (end > activeCount) {
end = activeCount;
}
Repository[] memory pagedRepos = new Repository[](end - offset);
for (uint256 i = offset; i < end; i++) {
pagedRepos[i - offset] = tempRepos[i];
}
return pagedRepos;
}
Comment on lines +175 to 204
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add input validation for pagination parameters.

The implementation correctly handles pagination logic, but missing input validation could lead to unexpected behavior:

  1. limit = 0 would create a zero-length array, which while not breaking, may not be the intended behavior.
  2. No upper bound on limit means callers could request excessively large arrays.

Apply this diff to add input validation:

 function getActiveRepositories(uint256 offset, uint256 limit) external view returns (Repository[] memory) {
+    require(limit > 0, "Limit must be greater than 0");
+    require(limit <= 100, "Limit exceeds maximum allowed");
     // Gather all active repositories
     uint256 activeCount = 0;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function getActiveRepositories(uint256 offset, uint256 limit) external view returns (Repository[] memory) {
// Gather all active repositories
uint256 activeCount = 0;
for (uint256 i = 1; i <= repoCounter; i++) {
if (repositories[i].isActive && repositories[i].maintainer != address(0)) {
activeCount++;
}
}
// Create array with exact size needed
Repository[] memory activeRepos = new Repository[](activeCount);
// Second pass: populate the array
Repository[] memory tempRepos = new Repository[](activeCount);
uint256 index = 0;
for (uint256 i = 1; i <= repoCounter; i++) {
if (repositories[i].isActive && repositories[i].maintainer != address(0)) {
activeRepos[index] = repositories[i];
tempRepos[index] = repositories[i];
index++;
}
}
return activeRepos;
// Pagination
if (offset >= activeCount) {
return new Repository[](0);
}
uint256 end = offset + limit;
if (end > activeCount) {
end = activeCount;
}
Repository[] memory pagedRepos = new Repository[](end - offset);
for (uint256 i = offset; i < end; i++) {
pagedRepos[i - offset] = tempRepos[i];
}
return pagedRepos;
}
function getActiveRepositories(uint256 offset, uint256 limit) external view returns (Repository[] memory) {
require(limit > 0, "Limit must be greater than 0");
require(limit <= 100, "Limit exceeds maximum allowed");
// Gather all active repositories
uint256 activeCount = 0;
for (uint256 i = 1; i <= repoCounter; i++) {
if (repositories[i].isActive && repositories[i].maintainer != address(0)) {
activeCount++;
}
}
Repository[] memory tempRepos = new Repository[](activeCount);
uint256 index = 0;
for (uint256 i = 1; i <= repoCounter; i++) {
if (repositories[i].isActive && repositories[i].maintainer != address(0)) {
tempRepos[index] = repositories[i];
index++;
}
}
// Pagination
if (offset >= activeCount) {
return new Repository[](0);
}
uint256 end = offset + limit;
if (end > activeCount) {
end = activeCount;
}
Repository[] memory pagedRepos = new Repository[](end - offset);
for (uint256 i = offset; i < end; i++) {
pagedRepos[i - offset] = tempRepos[i];
}
return pagedRepos;
}
🤖 Prompt for AI Agents 
In contracts/RepositoryRegistry.sol around lines 175-204, add validation for
pagination inputs: require limit > 0 (fail fast) and enforce an upper bound by
introducing a MAX_LIMIT constant (e.g., 100) and require(limit <= MAX_LIMIT);
also ensure offset handling remains safe by keeping the existing offset >=
activeCount early-return or, when computing end, cap end to offset + min(limit,
activeCount - offset) so callers cannot request more items than remain. Ensure
the MAX_LIMIT constant is declared near the top of the contract and update
pagination logic to use the validated/capped limit.


/**
Expand Down