Skip to content

add support for WriteLogonScript #259

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

Open
ncha-syn wants to merge 1 commit into
base: v4
Choose a base branch
from
Open

add support for WriteLogonScript #259

Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions src/CommonLib/Enums/EdgeNames.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public static class EdgeNames
public const string ReadGMSAPassword = "ReadGMSAPassword";
public const string AddMember = "AddMember";
public const string WriteSPN = "WriteSPN";
public const string WriteLogonScript = "WriteLogonScript";
public const string AddKeyCredentialLink = "AddKeyCredentialLink";
public const string SQLAdmin = "SQLAdmin";
public const string WriteAccountRestrictions = "WriteAccountRestrictions";
Expand Down
1 change: 1 addition & 0 deletions src/CommonLib/Processors/ACEGuids.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public class ACEGuids
public const string WriteMember = "bf9679c0-0de6-11d0-a285-00aa003049e2";
public const string WriteAllowedToAct = "3f78c3e5-f79a-46bd-a0b8-9d18116ddc79";
public const string WriteSPN = "f3a64788-5306-11d1-a9c5-0000f80367c1";
public const string WriteLogonScript = "bf9679a8-0de6-11d0-a285-00aa003049e2";
public const string AddKeyPrincipal = "5b47d60f-6090-40b2-9f37-2a4de88f3063";
public const string UserAccountRestrictions = "4c164200-20c0-11d0-a768-00aa006e0529";
public const string WriteGPLink = "f30e3bbe-9ff0-11d1-b603-0000f80367c1";
Expand Down
11 changes: 11 additions & 0 deletions src/CommonLib/Processors/ACLProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,17 @@ or Label.NTAuthStore
IsPermissionForOwnerRightsSid = isPermissionForOwnerRightsSid,
IsInheritedPermissionForOwnerRightsSid = isInheritedPermissionForOwnerRightsSid,
};
else if (objectType == Label.User && aceType == ACEGuids.WriteLogonScript)
yield return new ACE
{
PrincipalType = resolvedPrincipal.ObjectType,
PrincipalSID = resolvedPrincipal.ObjectIdentifier,
IsInherited = inherited,
RightName = EdgeNames.WriteLogonScript,
InheritanceHash = aceInheritanceHash,
IsPermissionForOwnerRightsSid = isPermissionForOwnerRightsSid,
IsInheritedPermissionForOwnerRightsSid = isInheritedPermissionForOwnerRightsSid,
};
Comment on lines +759 to +769
Copy link

@coderabbitai coderabbitai bot Nov 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Implementation correctly follows established ACE processing patterns.

The WriteLogonScript ACE handling mirrors the existing WriteSPN implementation and properly includes all necessary properties (inheritance tracking, owner rights handling, etc.). The logic is correctly scoped to User objects only, which aligns with the scriptPath attribute's applicability.

However, please verify that corresponding tests and documentation have been added for this new edge type. Based on the PR objectives, the checklist items for tests and documentation are not marked as complete.

🏁 Script executed:

#!/bin/bash
# Description: Search for existing tests for similar ACE types (WriteSPN) to identify test patterns
# Expected: Find test files that should be updated to include WriteLogonScript coverage

echo "=== Searching for WriteSPN tests that should be replicated for WriteLogonScript ==="
rg -i "WriteSPN" --type cs -g "*test*" -C 3

echo -e "\n=== Checking if WriteLogonScript tests exist ==="
rg -i "WriteLogonScript" --type cs -g "*test*" -C 3

echo -e "\n=== Searching for ACLProcessor test files ==="
fd -e cs -i "test" | rg -i "acl"

Length of output: 508

I found the test file. Let me search it directly for existing test patterns:

</script>

🏁 Script executed:

#!/bin/bash
# Search the test file for WriteSPN tests (the reference pattern)
echo "=== WriteSPN tests in ACLProcessorTest.cs ==="
rg -i "WriteSPN" test/unit/ACLProcessorTest.cs -C 5

echo -e "\n=== WriteLogonScript tests in ACLProcessorTest.cs ==="
rg -i "WriteLogonScript" test/unit/ACLProcessorTest.cs -C 5

echo -e "\n=== Checking implementation file for documentation ==="
rg -i "WriteLogonScript" src/CommonLib/Processors/ACLProcessor.cs -C 3

Length of output: 6071

Add unit tests and documentation for WriteLogonScript ACE handling.

WriteLogonScript tests are missing from test/unit/ACLProcessorTest.cs. WriteSPN—the referenced pattern—has multiple test cases (ACLProcessor_ProcessACL_GenericAll_WriteSPN, ACLProcessor_ProcessACL_GenericWrite_User_WriteSPN, etc.). Add equivalent test coverage for WriteLogonScript, including GenericAll and GenericWrite scenarios with User object type. Additionally, add inline documentation comments to the implementation at lines 759-769 in src/CommonLib/Processors/ACLProcessor.cs to align with established code documentation practices.

🤖 Prompt for AI Agents 
In src/CommonLib/Processors/ACLProcessor.cs around lines 759 to 769, add an
inline XML comment above the else-if handling ACEGuids.WriteLogonScript that
documents the mapping: for User object type the ACEGuids.WriteLogonScript maps
to EdgeNames.WriteLogonScript and the produced ACE sets PrincipalType,
PrincipalSID, IsInherited, RightName, InheritanceHash,
IsPermissionForOwnerRightsSid, and IsInheritedPermissionForOwnerRightsSid; in
test/unit/ACLProcessorTest.cs add unit tests mirroring the existing WriteSPN
cases (e.g. ACLProcessor_ProcessACL_GenericAll_WriteLogonScript,
ACLProcessor_ProcessACL_GenericWrite_User_WriteLogonScript and any other
parallel scenarios), following the same arrangements and assertions as the
WriteSPN tests to verify the ACE is yielded and all fields (PrincipalType,
PrincipalSID, IsInherited, RightName == EdgeNames.WriteLogonScript,
InheritanceHash and the owner-rights flags) match expected values.

else if (objectType == Label.Computer && aceType == ACEGuids.WriteAllowedToAct)
yield return new ACE {
PrincipalType = resolvedPrincipal.ObjectType,
Expand Down