-
Notifications
You must be signed in to change notification settings - Fork 1
Audit-log #11
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
base: main
Are you sure you want to change the base?
Audit-log #11
Conversation
scottluskcis
commented
Nov 19, 2025
- ability to look for users in audit log removed
- ability to add users back mistakenly removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds audit log functionality to identify users who were mistakenly removed from repositories and provides the ability to restore them. The implementation consists of two new commands that work together to analyze audit logs and perform restoration operations.
- Added
identify-removed-userscommand to analyze audit logs and identify removal candidates - Added
restore-removed-userscommand to restore users from CSV data - Added enterprise-level audit log support alongside organization-level support
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.ts | Registers the two new commands (identify-removed-users and restore-removed-users) |
| src/commands/identify-removed-users.ts | Implements command to identify removed users from audit logs and determine restoration eligibility |
| src/commands/restore-removed-users.ts | Implements command to restore removed users to repositories based on CSV input |
| src/commands/command-helpers.ts | Adds enterprise name option to shared command options |
| src/api/audit-log-events.ts | Provides generic audit log activity retrieval functionality supporting both org and enterprise levels |
| .vscode/launch.json | Adds debug configurations for the two new commands |
Comments suppressed due to low confidence (4)
src/commands/identify-removed-users.ts:4
- Unused import parseRepoListOption.
import {
createCommandWithSharedOptions,
parseRepoListOption,
} from './command-helpers.js';
src/commands/identify-removed-users.ts:14
- Unused import extractHeaders.
import {
initializeCsvFile,
appendRecordToCsv,
extractHeaders,
} from '../utils/csv.js';
src/commands/identify-removed-users.ts:16
- Unused import c.
import { c } from 'tar';
src/commands/identify-removed-users.ts:93
- Unused function getAdd.
async function getAdd() {}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| : `created:${created}` | ||
| : ''; | ||
|
|
||
| const repoPhrase = repoName ? `repo:${orgName}/${repoName}` : ''; |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential bug when repoName is provided without orgName. The repoPhrase construction assumes orgName is available when repoName is provided (line 78), but orgName is optional. This could result in an invalid phrase like repo:undefined/repoName if repoName is provided but orgName is not.
| const repoPhrase = repoName ? `repo:${orgName}/${repoName}` : ''; | |
| const repoPhrase = repoName && orgName ? `repo:${orgName}/${repoName}` : ''; |
| const identifyRemovedUsersCommand = createCommandWithSharedOptions( | ||
| 'identify-removed-users', | ||
| ) | ||
| .description('List audit log events for the specified organization') |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misleading command description. The description says "List audit log events for the specified organization" which is generic and doesn't clearly describe what this command actually does (identify users who were removed and determine if they should be restored). Consider a more specific description like "Identify removed users from audit log events and determine restoration candidates".
| .description('List audit log events for the specified organization') | |
| .description('Identify removed users from audit log events and determine restoration candidates for the specified organization') |
| async function getAdd() {} | ||
|
|
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty function that serves no purpose. This getAdd function is declared but never used and has no implementation. It should be removed.
| async function getAdd() {} |
| include?: 'all' | 'web' | 'git'; | ||
| type: 'org' | 'enterprise'; | ||
| }): AsyncGenerator<{ actor: string; action: string; date: Date; props: any }> { | ||
| if (type == 'org' && !orgName) { |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use strict equality operators. Should use === instead of == for type-safe comparison.
| } | ||
|
|
||
| // Check if user is already a direct collaborator | ||
| let isDirectCollaborator = false; |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable. The isDirectCollaborator variable is assigned a value but never read or used afterwards. Consider removing it.
| const phrase = actionPhrases.join(' ').trim(); | ||
|
|
||
| const url = | ||
| type == 'org' |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use strict equality operators. Should use === instead of == for type-safe comparison.
| type == 'org' | |
| type === 'org' |
| `User ${user} is a direct collaborator with ${currentPermission} permission on ${repoFullName}`, | ||
| ); | ||
|
|
||
| if (currentPermission === origPermission) { |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential bug in permission comparison. The currentPermission from the API may be in a different format (e.g., 'pull'/'push') than origPermission from audit logs (e.g., 'read'/'write'). The comparison at line 267 may incorrectly report permissions as different when they are semantically the same. Consider normalizing both permissions using mapPermissionToApi before comparison.
| let restorationCandidates = 0; | ||
| let skippedDueToSubsequentActivity = 0; | ||
|
|
||
| for (const { event, existing, existingInfo } of allRemovalEvents) { |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused variable existing.
| for (const { event, existing, existingInfo } of allRemovalEvents) { | |
| for (const { event, existingInfo } of allRemovalEvents) { |
| }); | ||
|
|
||
| // If we get here (no 404), they are a direct collaborator | ||
| isDirectCollaborator = true; |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value assigned to isDirectCollaborator here is unused.
| let restorationCandidates = 0; | ||
| let skippedDueToSubsequentActivity = 0; | ||
|
|
||
| for (const { event, existing, existingInfo } of allRemovalEvents) { |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For loop variable existing is not used in the loop body.