ADCS ESC14 Scenario A coverage #1660
Conversation
JonasBK
added
enhancement
WalkthroughAdds two AD relationship kinds ( Changes
Sequence Diagram(s)sequenceDiagram
%% Styling: subtle rects to highlight new/changed steps
participant User
participant UI
participant Backend
participant GraphSchema
User->>UI: Request edge help (e.g. WriteAltSecurityIdentities)
UI->>Backend: fetch edge metadata & scenario content
Backend->>GraphSchema: resolve relationship kind (includes new kinds)
GraphSchema-->>Backend: return kind metadata
Backend-->>UI: return aggregated help + analysis results
UI->>UI: render help text, abuse scenarios, references (ADCS ESC14 components)
UI-->>User: display enriched help content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/Opsec.tsx (1)
17-30: Same content as the WritePublicInformation Opsec – DRY violationSee comment on the sibling file. Please extract the common text into a single source to keep the two edge kinds in sync.
🧹 Nitpick comments (6)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/Opsec.tsx (1)
17-30: Duplicate component – consider centralising to avoid driftThe body text and implementation are byte-for-byte identical to
HelpTexts/WriteAltSecurityIdentities/Opsec.tsx.
If the wording ever needs to change, maintaining two copies is error-prone.A small shared helper (e.g.
components/HelpTexts/Common/OpsecCAIssuedCertWarning.tsx) that both relationship-specific wrappers re-export would eliminate the duplication.packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/WriteAltSecurityIdentities.tsx (1)
23-29: Add an explicit type for the descriptor objectDefining the shape helps catch typos in the keys (
general,windowsAbuse, …) at compile-time and improves IDE completion.-const WriteAltSecurityIdentities = { +const WriteAltSecurityIdentities: Record< + 'general' | 'windowsAbuse' | 'linuxAbuse' | 'opsec' | 'references', + FC +> = { general: General, windowsAbuse: WindowsAbuse, linuxAbuse: LinuxAbuse, opsec: Opsec, references: References, };packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/LinuxAbuse.tsx (1)
205-206: Same JSX inserted in three branches – extract to reduce duplication
<AdcsEsc14ScenarioALinux />is rendered in everyUser,Computer (haslaps), andComputer (!haslaps)branch.
A single render after theswitch(or factoring common code into a helper) would shorten the component and make it less error-prone.Also applies to: 324-325, 399-400
packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/LinuxAbuse.tsx (1)
22-22: Address unused props parameter.The
EdgeInfoPropsparameter is declared but not used in the component. Either utilize these props for dynamic content or remove the parameter if not needed.If the props aren't needed:
-const LinuxAbuse: FC<EdgeInfoProps> = () => { +const LinuxAbuse: FC = () => {Or if they should be used, consider utilizing them for personalized content based on the edge information.
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/WindowsAbuse.tsx (1)
144-147: Consider extracting duplicated explanatory text to improve maintainability.The same explanatory text about altSecurityIdentities attribute enabling ADCS ESC14 Scenario A attack is repeated identically in three locations. This creates maintenance overhead if the text needs to be updated.
Consider extracting this into a reusable constant or component:
+const ADCS_ESC14_ALT_SECURITY_TEXT = "The permission also grants write access to the \"altSecurityIdentities\" attribute, which enables an ADCS ESC14 Scenario A attack."; // Then replace each occurrence with: - <Typography variant='body2'> - The permission also grants write access to the "altSecurityIdentities" attribute, which enables - an ADCS ESC14 Scenario A attack. - </Typography> + <Typography variant='body2'> + {ADCS_ESC14_ALT_SECURITY_TEXT} + </Typography>Also applies to: 309-312, 489-492
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
63-83: Consider security implications of exposing detailed attack commands.While this is educational content for a legitimate security tool, the specific command examples with actual tools (certipy, openssl) provide step-by-step attack instructions. Ensure this aligns with responsible disclosure practices and the tool's intended defensive use.
The technical content appears accurate, but consider adding a disclaimer about ethical use or intended defensive purposes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
cmd/api/src/test/integration/harnesses/esc10aprincipalharness.svgis excluded by!**/*.svgcmd/api/src/test/integration/harnesses/esc9aprincipalharness.svgis excluded by!**/*.svg
📒 Files selected for processing (40)
cmd/api/src/analysis/ad/adcs_integration_test.go(2 hunks)cmd/api/src/test/integration/harnesses.go(6 hunks)cmd/api/src/test/integration/harnesses/esc10aprincipalharness.json(4 hunks)cmd/api/src/test/integration/harnesses/esc9aprincipalharness.json(4 hunks)packages/cue/bh/ad/ad.cue(4 hunks)packages/go/analysis/ad/queries.go(1 hunks)packages/go/graphschema/ad/ad.go(2 hunks)packages/go/graphschema/common/common.go(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/index.ts(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/LinuxAbuse.tsx(4 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/WindowsAbuse.tsx(7 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/LinuxAbuse.tsx(3 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/WindowsAbuse.tsx(5 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/LinuxAbuse.tsx(4 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/WindowsAbuse.tsx(7 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/General.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/LinuxAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/Opsec.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/WindowsAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/WriteAltSecurityIdentities.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/LinuxAbuse.tsx(4 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/WindowsAbuse.tsx(7 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/LinuxAbuse.tsx(4 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/WindowsAbuse.tsx(7 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/General.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/LinuxAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/Opsec.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/WindowsAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/WritePublicInformation.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx(2 hunks)packages/javascript/bh-shared-ui/src/edgeTypes.tsx(1 hunks)packages/javascript/bh-shared-ui/src/graphSchema.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (8)
packages/go/graphschema/common/common.go (1)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
cmd/api/src/test/integration/harnesses/esc10aprincipalharness.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
cmd/api/src/test/integration/harnesses.go (3)
undefined
<retrieved_learning>
Learnt from: elikmiller
PR: #1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/*.go are generated from CUE schemas. When just prepare-for-codereview is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
</retrieved_learning>
<retrieved_learning>
Learnt from: mvlipka
PR: #1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
</retrieved_learning>
<retrieved_learning>
Learnt from: JonasBK
PR: #1434
File: packages/go/analysis/ad/gpos.go:102-126
Timestamp: 2025-06-18T08:27:18.317Z
Learning: In Active Directory's containment hierarchy, each user/computer has exactly one direct parent container, forming a tree structure. When processing GPO edges in packages/go/analysis/ad/gpos.go, the fetchDirectChildUsersAndComputers function only returns direct children, ensuring each user/computer is processed exactly once by its immediate parent container, eliminating the need for deduplication logic.
</retrieved_learning>
packages/cue/bh/ad/ad.cue (1)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
cmd/api/src/analysis/ad/adcs_integration_test.go (2)
undefined
<retrieved_learning>
Learnt from: superlinkx
PR: #1503
File: cmd/api/src/services/job/jobs_test.go:19-143
Timestamp: 2025-05-27T16:58:33.295Z
Learning: Tests in cmd/api/src/services/job/jobs_test.go have been found to be flaky in the past and are due for rewrite. They should be skipped with t.Skip() until they can be properly rewritten.
</retrieved_learning>
<retrieved_learning>
Learnt from: elikmiller
PR: #1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/*.go are generated from CUE schemas. When just prepare-for-codereview is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
</retrieved_learning>
packages/go/graphschema/ad/ad.go (1)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
cmd/api/src/test/integration/harnesses/esc9aprincipalharness.json (1)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
packages/go/analysis/ad/queries.go (1)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
🧬 Code Graph Analysis (19)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
packages/go/graphschema/ad/ad.go (2)
WriteAltSecurityIdentities(128-128)WritePublicInformation(129-129)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/LinuxAbuse.tsx (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
AdcsEsc14ScenarioALinux(21-114)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/LinuxAbuse.tsx (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
AdcsEsc14ScenarioALinux(21-114)
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/WindowsAbuse.tsx (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
AdcsEsc14ScenarioAWindows(116-221)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/WindowsAbuse.tsx (2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
EdgeInfoProps(145-154)packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
AdcsEsc14ScenarioAWindows(116-221)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/WindowsAbuse.tsx (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
AdcsEsc14ScenarioAWindows(116-221)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/WriteAltSecurityIdentities.tsx (1)
packages/go/graphschema/ad/ad.go (1)
WriteAltSecurityIdentities(128-128)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/General.tsx (2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
EdgeInfoProps(145-154)packages/javascript/bh-shared-ui/src/components/HelpTexts/utils.ts (2)
groupSpecialFormat(19-26)typeFormat(28-45)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/WritePublicInformation.tsx (1)
packages/go/graphschema/ad/ad.go (1)
WritePublicInformation(129-129)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/LinuxAbuse.tsx (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
AdcsEsc14ScenarioALinux(21-114)
cmd/api/src/test/integration/harnesses.go (3)
cmd/api/src/test/integration/harnesses/utils.go (1)
Node(38-44)packages/go/graphschema/ad/ad.go (2)
Owns(44-44)WritePublicInformation(129-129)packages/go/graphschema/azure/azure.go (1)
Owns(72-72)
cmd/api/src/analysis/ad/adcs_integration_test.go (1)
cmd/api/src/test/integration/harnesses.go (2)
ESC9aPrincipalHarness(2200-2217)ESC10aPrincipalHarness(3742-3759)
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/WindowsAbuse.tsx (2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/index.ts (1)
AdcsEsc14ScenarioAWindows(17-17)packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
AdcsEsc14ScenarioAWindows(116-221)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/LinuxAbuse.tsx (2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
EdgeInfoProps(145-154)packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
AdcsEsc14ScenarioALinux(21-114)
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/General.tsx (2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
EdgeInfoProps(145-154)packages/javascript/bh-shared-ui/src/components/HelpTexts/utils.ts (2)
groupSpecialFormat(19-26)typeFormat(28-45)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/index.ts (2)
AdcsEsc14ScenarioALinux(17-17)AdcsEsc14ScenarioAWindows(17-17)packages/go/headers/headers.go (1)
Link(112-112)
packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/WindowsAbuse.tsx (2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/index.ts (1)
AdcsEsc14ScenarioAWindows(17-17)packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
AdcsEsc14ScenarioAWindows(116-221)
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/LinuxAbuse.tsx (2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/index.ts (1)
AdcsEsc14ScenarioALinux(17-17)packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
AdcsEsc14ScenarioALinux(21-114)
packages/go/analysis/ad/queries.go (4)
packages/go/graphschema/ad/ad.go (7)
Computer(30-30)GenericAll(45-45)GenericWrite(46-46)Owns(44-44)WriteOwner(47-47)WriteDACL(48-48)WritePublicInformation(129-129)cmd/api/src/test/integration/harnesses/harnessgen.py (1)
Relationship(147-154)cmd/api/src/test/integration/harnesses/utils.go (1)
Relationship(46-53)packages/go/graphschema/azure/azure.go (1)
Owns(72-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-analysis
- GitHub Check: build-ui
- GitHub Check: run-tests
🔇 Additional comments (88)
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/References.tsx (1)
165-171: LGTM! Well-positioned reference addition.The new ADCS ESC14 reference link is correctly formatted and logically placed after the ESC13 reference, maintaining chronological order. The link follows the established pattern with proper security attributes.
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/LinuxAbuse.tsx (2)
19-19: Good import addition for ESC14 scenario.The import follows the established pattern and brings in the necessary component for ADCS ESC14 scenario documentation.
113-113: Appropriate integration of ESC14 scenario for both User and Computer cases.The
AdcsEsc14ScenarioALinuxcomponent is correctly added to both 'User' and 'Computer' switch cases, which makes sense since ESC14 can apply to both target types. The placement after the pyWhisker documentation maintains logical flow of the abuse scenarios.Also applies to: 167-167
packages/javascript/bh-shared-ui/src/edgeTypes.tsx (1)
99-99: Correct categorization of new edge types.The new
WriteAltSecurityIdentitiesandWritePublicInformationedge types are appropriately categorized under "Advanced Object Manipulation" and maintain alphabetical ordering within the subcategory. These align with the backend schema changes for ESC14 support.Also applies to: 101-101
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/References.tsx (1)
113-119: Excellent consistency in reference documentation.The ADCS ESC14 reference addition maintains consistency with other help text components and follows the established pattern. The positioning after ESC13 and formatting are correct.
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (2)
134-134: Proper component imports for new edge types.The imports for
WriteAltSecurityIdentitiesandWritePublicInformationfollow the established pattern and align with the backend schema definitions for these new relationship kinds.Also applies to: 142-142
281-282: Correct registration of new help text components.The new components are properly added to the
EdgeInfoComponentsobject, making them available for use throughout the UI. This completes the integration of ESC14 scenario documentation into the help text system.cmd/api/src/analysis/ad/adcs_integration_test.go (4)
1275-1275: LGTM! Correctly updated expected result count for ESC9a test.The test now expects 7 results instead of 6, which aligns with the addition of Group6 to the ESC9a attack path as shown in the harness definition.
1282-1282: LGTM! Added assertion for Group6 in ESC9a results.This assertion correctly verifies that Group6 is now included in the ESC9a attack path results, consistent with the test harness changes mentioned in the relevant code snippets.
2662-2662: LGTM! Correctly updated expected result count for ESC10a test.The test now expects 7 results instead of 6, which aligns with the addition of Group6 to the ESC10a attack path as shown in the harness definition.
2669-2669: LGTM! Added assertion for Group6 in ESC10a results.This assertion correctly verifies that Group6 is now included in the ESC10a attack path results, consistent with the test harness changes and the introduction of new relationship types like WritePublicInformation.
packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/LinuxAbuse.tsx (2)
19-19: LGTM: Clean import addition.The import follows the established pattern and correctly references the new ADCS ESC14 component.
205-205: LGTM: Consistent integration across all target types.The
AdcsEsc14ScenarioALinuxcomponent is systematically integrated in all appropriate locations (User, Computer with LAPS, Computer without LAPS) immediately after the Shadow Credentials documentation. This placement is logical and maintains consistency across the abuse scenarios.Also applies to: 324-324, 398-398
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/index.ts (1)
1-18: LGTM: Standard barrel export implementation.Clean implementation following established patterns with proper licensing and straightforward re-exports for the ADCS ESC14 components.
packages/go/graphschema/common/common.go (1)
43-43: LGTM: Generated schema updates are correct.The addition of
ad.WriteAltSecurityIdentitiesandad.WritePublicInformationto both inbound and outbound relationship kinds is properly generated from the CUE schemas. The placement in the lists is appropriate and consistent with the schema definitions.Also applies to: 46-46
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/LinuxAbuse.tsx (2)
19-19: LGTM: Consistent import pattern.The import follows the same pattern established in other help text components.
158-158: LGTM: Systematic integration matches other components.The integration pattern perfectly matches the implementation in the Owns component, showing consistent approach across different permission types. The placement after Shadow Credentials documentation is logical and maintains the flow of attack scenarios.
Also applies to: 279-279, 333-333
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/LinuxAbuse.tsx (1)
1-33: LGTM: Well-structured new component.Clean implementation following established patterns with:
- Proper Apache 2.0 license header
- Standard React functional component with TypeScript
- Appropriate Material-UI Typography usage
- Clear, concise description of the attack vector
- Logical composition with the detailed ADCS ESC14 component
The component effectively bridges the specific WriteAltSecurityIdentities permission to the broader ADCS ESC14 Scenario A attack documentation.
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/LinuxAbuse.tsx (1)
17-20: Import path validatedThe barrel file at
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/index.tsexists and re-exportsAdcsEsc14ScenarioALinux, so the import inWriteDacl/LinuxAbuse.tsxresolves correctly.• packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/index.ts → exports
AdcsEsc14ScenarioALinux
• packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/LinuxAbuse.tsx →import { AdcsEsc14ScenarioALinux } from '../AdcsEsc14ScenarioA';is validpackages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/References.tsx (1)
103-108: LGTM – useful reference addedThe ESC14 abuse-technique link rounds out the reference list nicely.
packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/References.tsx (1)
147-153: LGTM: Reference link addition follows established patterns.The ADCS ESC14 reference link is properly positioned after the ESC13 link and maintains consistent formatting with other external references. The placement is logical and the implementation follows the established pattern.
packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/WritePublicInformation.tsx (1)
17-31: LGTM: Standard help text aggregation pattern implemented correctly.The component follows the established pattern for aggregating help text components. The import structure, object creation, and default export are all consistent with other similar components in the codebase.
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/LinuxAbuse.tsx (2)
19-19: LGTM: Import added correctly for ADCS ESC14 scenario component.
218-218: LGTM: Strategic placement of ADCS ESC14 scenario component.The
AdcsEsc14ScenarioALinuxcomponent is consistently placed after the shadow credentials attack sections across all relevant target types. This placement is logical since ADCS ESC14 scenarios often build upon or complement shadow credentials attacks.Also applies to: 344-344, 426-426
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/References.tsx (1)
158-164: LGTM: Consistent reference link addition.The ADCS ESC14 reference link follows the same pattern as other similar additions in this PR, maintaining proper formatting and logical positioning within the reference list.
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/WindowsAbuse.tsx (1)
22-31: LGTM: Well-focused component implementation.The component correctly implements the Windows abuse scenario for
WriteAltSecurityIdentities. While it acceptsEdgeInfoPropsbut doesn't use them, this follows the established pattern for interface consistency across help text components. The integration withAdcsEsc14ScenarioAWindowsis appropriate and the content is well-scoped.packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/References.tsx (1)
20-97: LGTM! Well-structured reference component.The References component is correctly implemented with appropriate security-related links and proper Material-UI usage.
cmd/api/src/test/integration/harnesses/esc10aprincipalharness.json (1)
190-280: LGTM! Test harness updates are appropriate.The structural changes to add Group7, reposition Group6, and update relationships align with the test scenario requirements. The JSON is properly formatted and the changes are consistent.
Also applies to: 423-493
packages/cue/bh/ad/ad.cue (2)
1636-1644: LGTM! New relationship kinds are properly defined.The WriteAltSecurityIdentities and WritePublicInformation relationship kinds follow the established schema patterns correctly.
1732-1733: LGTM! Relationship kinds are consistently added to all relevant lists.The new relationship kinds are appropriately included in RelationshipKinds, ACLRelationships, and SharedRelationshipKinds lists.
Also applies to: 1766-1767, 1825-1826
packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/LinuxAbuse.tsx (1)
24-38: LGTM! Well-structured abuse scenario documentation.The component effectively explains the security risks and provides clear guidance on ADCS ESC14 Scenario A and Kerberoasting attacks with appropriate external references.
packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/WindowsAbuse.tsx (7)
19-19: LGTM: Import statement is correctly added.The import for
AdcsEsc14ScenarioAWindowsfollows the existing import pattern and is placed appropriately with other component imports.
98-101: LGTM: Clear and informative description of the new attack vector.The added text effectively explains that GenericAll permission grants write access to the "altSecurityIdentities" attribute, which enables the ADCS ESC14 Scenario A attack. This addition is consistent with how other attack vectors are documented in this section.
178-178: LGTM: Component integration is well-positioned.The
AdcsEsc14ScenarioAWindowscomponent is appropriately placed after the Shadow Credentials section and before the Targeted Kerberoast section, maintaining logical flow of attack methods.
241-244: LGTM: Consistent description for Computer objects with LAPS.The text addition maintains consistency with the User case description while being appropriately placed within the Computer (with LAPS) section.
381-381: LGTM: Component placement follows the established pattern.The component is correctly positioned in the Computer with LAPS case, maintaining consistency with the User case placement.
399-402: LGTM: Complete coverage across all Computer object scenarios.The description is consistently added to the Computer without LAPS case, ensuring comprehensive documentation of the attack vector across all relevant target types.
489-489: LGTM: Final component integration completes the implementation.The component is correctly included in the Computer without LAPS case, completing the comprehensive integration across all relevant GenericAll abuse scenarios.
packages/javascript/bh-shared-ui/src/graphSchema.ts (3)
157-158: LGTM: New relationship kinds are correctly added to the enum.The
WriteAltSecurityIdentitiesandWritePublicInformationenum values are properly added to theActiveDirectoryRelationshipKindenum, following the existing naming convention and placement.
330-333: LGTM: Display function cases are correctly implemented.The switch cases for the new relationship kinds correctly return their string representations, following the established pattern of returning the enum value as a string.
814-815: LGTM: Pathfinding edges inclusion is appropriate.Both new relationship kinds are correctly included in the
ActiveDirectoryPathfindingEdgesfunction, which indicates they are relevant for graph traversal and attack path analysis. The placement maintains alphabetical ordering within the function.packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/WindowsAbuse.tsx (1)
1-43: LGTM! Well-structured help text component with accurate security information.The component follows established patterns, properly imports dependencies, and provides clear explanations of attack scenarios. The unused
EdgeInfoPropsappears intentional for this general help text component.packages/go/analysis/ad/queries.go (1)
1812-1820: LGTM! Proper conditional filtering for ESC14 scenarios.The conditional logic correctly differentiates between Scenario A and B by including
WritePublicInformationonly whenscenarioB == false. This aligns with the ADCS ESC14 Scenario A attack path requirements.packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/WindowsAbuse.tsx (5)
19-19: Good addition of ADCS ESC14 attack documentation.The import of
AdcsEsc14ScenarioAWindowscomponent properly integrates the new attack scenario into the help text system.
77-80: Clear explanation of altSecurityIdentities write access risk.The text accurately describes how GenericWrite permission enables ADCS ESC14 Scenario A attack through write access to the
altSecurityIdentitiesattribute.
102-102: Well-positioned ADCS ESC14 component integration.The placement after the Shadow Credentials section logically flows with the attack scenario explanations.
189-193: Consistent messaging across User and Computer scenarios.The explanation maintains consistency with the User scenario while appropriately contextualizing it for Computer objects.
279-279: Appropriate placement after Resource-Based Constrained Delegation.The ADCS ESC14 component is well-positioned after the existing attack scenarios for Computer objects.
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/General.tsx (1)
1-41: Excellent implementation of WriteAltSecurityIdentities help text component.The component follows established patterns, properly utilizes formatting utilities (
groupSpecialFormat,typeFormat), and provides clear, accurate information about thealtSecurityIdentitiesattribute and explicit certificate mappings. The structure and content are well-designed for user understanding.packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/WindowsAbuse.tsx (2)
19-19: LGTM: Import statement is correctly added.The import for
AdcsEsc14ScenarioAWindowsfrom the relative path is properly structured and follows the existing import pattern.
224-224: LGTM: Component placement is well-integrated.The
AdcsEsc14ScenarioAWindowscomponents are strategically placed within the appropriate abuse scenario sections, providing users with comprehensive attack information in context.Also applies to: 449-449, 579-579
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/WindowsAbuse.tsx (7)
19-19: LGTM: Clean import addition.The import for
AdcsEsc14ScenarioAWindowsfollows the established pattern and is correctly placed.
195-198: LGTM: Clear and accurate content addition.The explanation of write access to the "altSecurityIdentities" attribute is accurate and appropriately positioned within the GenericAll permission context for user objects.
275-275: LGTM: Appropriate component placement.The
AdcsEsc14ScenarioAWindowscomponent is well-positioned after the Shadow Credentials attack section, providing additional attack vector information for user targets.
383-386: LGTM: Consistent content pattern.The altSecurityIdentities explanation for computer objects with LAPS follows the same clear and accurate pattern as the user object section.
523-523: LGTM: Consistent component placement.The component is appropriately placed at the end of the computer object abuse section (with LAPS), maintaining consistency with the user object section structure.
587-590: LGTM: Consistent content across computer object variations.The altSecurityIdentities explanation is consistently applied to computer objects without LAPS, ensuring comprehensive coverage.
677-677: LGTM: Complete coverage maintained.The component is consistently included in both computer object scenarios (with and without LAPS), ensuring users have access to the ADCS ESC14 information regardless of the LAPS configuration.
packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/General.tsx (6)
1-16: LGTM: Standard license header.The copyright header follows the established pattern for 2025 Specter Ops files.
17-20: LGTM: Clean and appropriate imports.The imports are minimal and correct:
- Material-UI Typography for consistent styling
- React FC type for functional component
- EdgeInfoProps from the index for proper typing
- Utility functions for consistent formatting
22-28: LGTM: Well-structured component definition.The component properly:
- Uses TypeScript with EdgeInfoProps interface
- Destructures necessary props (sourceName, sourceType, targetName, targetType)
- Uses utility functions for consistent formatting
- Provides clear explanation of the WritePublicInformation relationship
30-35: LGTM: Accurate and informative content.The explanation of altSecurityIdentities attribute is technically accurate and well-written. It clearly explains:
- The concept of explicit certificate mappings
- How they work as an alternative to normal certificate-to-account mapping
- The security implications for authentication
37-39: LGTM: Complete attribute coverage.Including the servicePrincipalName (SPN) attribute ensures users understand the full scope of the Public-Information property set that can be modified.
44-45: LGTM: Standard export pattern.The default export follows the established pattern for help text components.
packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/WindowsAbuse.tsx (7)
19-19: LGTM! Import statement is correct.The import correctly references the
AdcsEsc14ScenarioAWindowscomponent from the expected location.
163-166: LGTM! Clear and accurate explanation.The explanatory text accurately describes that GenericAll grants write access to the "altSecurityIdentities" attribute, enabling the ADCS ESC14 Scenario A attack. The placement after the Shadow Credentials explanation is logical.
243-243: LGTM! Component placement enhances user experience.The
AdcsEsc14ScenarioAWindowscomponent is well-positioned after the Shadow Credentials attack section, providing users with comprehensive attack information in a logical flow.
333-336: LGTM! Consistent implementation across target types.The explanatory text for Computer objects with LAPS is identical to the User case, maintaining consistency in how the ADCS ESC14 Scenario A attack is described.
473-473: LGTM! Strategic component placement.The component is appropriately placed at the end of the Computer with LAPS section, providing complete attack coverage for this scenario.
518-521: LGTM! Maintains consistency across Computer scenarios.The explanatory text for Computer objects without LAPS matches the pattern established in other sections, ensuring a uniform user experience.
608-608: LGTM! Complete coverage across all Computer scenarios.The component placement ensures that both Computer scenarios (with and without LAPS) provide the same comprehensive ADCS ESC14 Scenario A attack information.
cmd/api/src/test/integration/harnesses.go (6)
2212-2212: LGTM: Group7 field addition is consistent.The addition of
Group7field follows the same pattern as other group fields in the ESC9aPrincipalHarness struct.
2244-2244: LGTM: Group7 instance creation follows established pattern.The creation of
Group7as an Active Directory group follows the same initialization pattern as other groups in the harness.
2260-2261: LGTM: Relationship changes align with ADCS ESC14 implementation.The changes correctly implement:
- Group5->User1 relationship change from
WriteOwnertoOwns- New Group6->User1 relationship with
WritePublicInformationpermissionBoth relationship types are properly defined in the AD schema and support the new ADCS ESC14 Scenario A coverage.
3755-3756: LGTM: Group field additions are consistent in ESC10aPrincipalHarness.The addition of
Group6andGroup7fields follows the same pattern as the ESC9aPrincipalHarness and maintains consistency across both test harnesses.
3786-3787: LGTM: Group instance creation is consistent.The creation of
Group6andGroup7instances follows the established pattern and maintains consistency with the ESC9aPrincipalHarness implementation.
3801-3802: LGTM: Relationship changes maintain consistency across harnesses.The relationship changes are identical to those in the ESC9aPrincipalHarness, ensuring consistent test coverage for the new ADCS ESC14 Scenario A functionality across both ESC9a and ESC10a test cases.
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (4)
17-20: LGTM: Clean imports and proper component setup.The imports are appropriate for a React help text component using Material UI. The inclusion of
CodeControllersuggests proper code formatting capabilities.
21-58: Comprehensive and technically accurate ADCS ESC14 Scenario A explanation.The component provides detailed and accurate information about:
- Certificate requirements for the attack
- EKU values that enable domain authentication
- UPN mapping considerations and workarounds
The content is well-structured and educational, appropriate for a security analysis tool.
88-112: LDAP modification examples are technically sound.The LDAP modify commands correctly demonstrate:
- Proper DN structure and syntax
- Correct attribute modification (altSecurityIdentities)
- Appropriate X509 certificate mapping format
- Cleanup procedures after abuse
116-221: Windows variant provides consistent platform-specific guidance.The Windows component mirrors the Linux approach with appropriate Windows-specific tools:
- Certify.exe for certificate enrollment
- certutil.exe for certificate manipulation
- PowerShell cmdlets for LDAP operations
- Rubeus for Kerberos ticket requests
The content maintains consistency with the Linux variant while using platform-appropriate tooling.
packages/go/graphschema/ad/ad.go (3)
128-129: New AD relationship kinds properly defined.The addition of
WriteAltSecurityIdentitiesandWritePublicInformationasgraph.StringKindconstants follows the established pattern and naming conventions used throughout the schema.
1090-1099: New relationship kinds properly integrated into all enumeration functions.Both
WriteAltSecurityIdentitiesandWritePublicInformationare correctly included in:
ACLRelationships()- appropriate as these represent ACL-based permissionsPathfindingRelationships()- correct for attack path analysisInboundRelationshipKinds()andOutboundRelationshipKinds()- proper bidirectional supportThe integration follows the established pattern and maintains consistency with existing relationship kinds.
1087-1087: Verified: New relationship kinds included in all enumeration functionsWriteAltSecurityIdentities and WritePublicInformation are present in Relationships(), ACLRelationships(), PathfindingRelationships(), InboundRelationshipKinds(), and OutboundRelationshipKinds(), confirming consistent integration across the board.
cmd/api/src/test/integration/harnesses/esc9aprincipalharness.json (4)
193-202: Node repositioning and renaming executed cleanly.The repositioning of the former Group6 to Group7 maintains proper JSON structure and styling consistency. The coordinate changes and caption update are properly formatted.
269-281: New Group6 node properly added with consistent styling.The new Group6 node follows the established JSON structure with:
- Proper positioning coordinates
- Consistent styling matching other group nodes
- Correct node color and formatting
479-494: New relationships properly integrate WritePublicInformation testing.The addition of two new relationships effectively tests the new functionality:
ADCSESC9arelationship from Group6 to Domain (n23)WritePublicInformationrelationship from Group6 to User1 (n24)This creates a test scenario where Group6 has both ADCS ESC9a capability and write access to public information attributes, properly exercising the new relationship type.
425-430: Double-check relationship type change in ESC9a principal harness
Our search didn’t reveal any tests explicitly assertingWriteOwneror referencing edge n16 by type, but the loader uses the JSONtypefield to build the graph. Please verify that changing n16 fromWriteOwnertoOwns:
- Still matches the intended ESC9a scenario in
cmd/api/src/test/integration/harnesses/esc9aprincipalharness.json(around lines 425–430).- Doesn’t break any code paths by running the AD integration suite (
go test ./cmd/api/src/analysis/ad).- Leaves no remaining
"type": "WriteOwner"entries in other harness files or test code.
...s/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/References.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (2)
21-114: Well-structured help text component with comprehensive attack documentation.The component provides detailed and technically accurate instructions for the ADCS ESC14 Scenario A attack on Linux systems. The content is well-organized with clear requirements, step-by-step procedures, and proper cleanup instructions.
Consider consistent code snippet formatting:
The component mixes
Typography component={'pre'}andCodeControllerfor displaying code snippets. Consider usingCodeControllerconsistently for better maintainability and user experience.Static analysis false positive:
The static analysis tool flagged line 90 as a potential API key, but this is actually an example SHA1 certificate fingerprint used in documentation (
f61331a504cff8cb5e60c269632c31aa3032a54a). This is not a security concern.
116-221: Comprehensive Windows-specific attack documentation with appropriate tooling.The component provides thorough coverage of the ADCS ESC14 Scenario A attack for Windows environments, using appropriate tools like Certify.exe, certutil.exe, PowerShell cmdlets, and Rubeus. The content maintains consistency with the Linux version while adapting to Windows-specific approaches.
Minor style inconsistency:
The Windows component uses an explicit
returnstatement while the Linux component uses implicit return. Consider maintaining consistency between the two components.Static analysis false positives:
The static analysis tool flagged lines 196 and 216 as potential API keys, but these are example SHA1 certificate fingerprints used in documentation (
ef9375785421d3ad286d8bdeb166f0f697266992). These are not security concerns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
cmd/api/src/test/integration/harnesses/esc10aprincipalharness.svgis excluded by!**/*.svgcmd/api/src/test/integration/harnesses/esc9aprincipalharness.svgis excluded by!**/*.svg
📒 Files selected for processing (40)
cmd/api/src/analysis/ad/adcs_integration_test.go(2 hunks)cmd/api/src/test/integration/harnesses.go(6 hunks)cmd/api/src/test/integration/harnesses/esc10aprincipalharness.json(4 hunks)cmd/api/src/test/integration/harnesses/esc9aprincipalharness.json(4 hunks)packages/cue/bh/ad/ad.cue(4 hunks)packages/go/analysis/ad/queries.go(1 hunks)packages/go/graphschema/ad/ad.go(2 hunks)packages/go/graphschema/common/common.go(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/index.ts(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/LinuxAbuse.tsx(4 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/WindowsAbuse.tsx(7 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/LinuxAbuse.tsx(3 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/WindowsAbuse.tsx(5 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/LinuxAbuse.tsx(4 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/WindowsAbuse.tsx(7 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/General.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/LinuxAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/Opsec.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/WindowsAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/WriteAltSecurityIdentities.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/LinuxAbuse.tsx(4 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/WindowsAbuse.tsx(7 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/LinuxAbuse.tsx(4 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/WindowsAbuse.tsx(7 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/General.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/LinuxAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/Opsec.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/References.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/WindowsAbuse.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/WritePublicInformation.tsx(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx(2 hunks)packages/javascript/bh-shared-ui/src/edgeTypes.tsx(1 hunks)packages/javascript/bh-shared-ui/src/graphSchema.ts(3 hunks)
✅ Files skipped from review due to trivial changes (8)
- packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/LinuxAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/Opsec.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/WritePublicInformation.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/References.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/References.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/References.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/General.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/WriteAltSecurityIdentities.tsx
🚧 Files skipped from review as they are similar to previous changes (31)
- packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/References.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/index.ts
- packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/References.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/LinuxAbuse.tsx
- packages/javascript/bh-shared-ui/src/edgeTypes.tsx
- packages/go/graphschema/common/common.go
- packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/LinuxAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/References.tsx
- cmd/api/src/test/integration/harnesses/esc10aprincipalharness.json
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/LinuxAbuse.tsx
- cmd/api/src/analysis/ad/adcs_integration_test.go
- packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/WindowsAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericAll/WindowsAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/LinuxAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/GenericWrite/LinuxAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/WindowsAbuse.tsx
- packages/go/graphschema/ad/ad.go
- cmd/api/src/test/integration/harnesses.go
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/WindowsAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/Owns/WindowsAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/WindowsAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteOwner/LinuxAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/General.tsx
- packages/go/analysis/ad/queries.go
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteAltSecurityIdentities/WindowsAbuse.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/References.tsx
- packages/javascript/bh-shared-ui/src/components/HelpTexts/WritePublicInformation/Opsec.tsx
- packages/cue/bh/ad/ad.cue
- packages/javascript/bh-shared-ui/src/graphSchema.ts
- cmd/api/src/test/integration/harnesses/esc9aprincipalharness.json
🧰 Additional context used
🧠 Learnings (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
Learnt from: JonasBK
PR: #1671
File: packages/go/analysis/ad/esc16.go:65-69
Timestamp: 2025-07-10T14:31:01.781Z
Learning: In BloodHound ADCS ESC analysis functions (PostADCSESC1, PostADCSESC4, PostADCSESC6, PostADCSESC9, PostADCSESC10, PostADCSESC13, PostADCSESC16, etc.), the established pattern for handling cert template validation errors and user filtering errors is to use slog.WarnContext to log warnings and continue processing with the next template, rather than returning errors to the caller. This pattern is consistently applied across all ADCS ESC edge implementations.
🪛 Gitleaks (8.27.2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx
90-90: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
110-110: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
196-196: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
216-216: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: build-ui
- GitHub Check: run-tests
- GitHub Check: run-tests
- GitHub Check: run-analysis
🔇 Additional comments (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
1-20: LGTM! Standard copyright header and appropriate imports.The copyright header follows the standard Apache 2.0 format and the imports are well-organized and appropriate for the component's functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
120-156: Same nested-paragraph issue in the Windows variant – apply the fix here as well.The
AdcsEsc14ScenarioAWindowscomponent repeats the exact<Typography>/<p>nesting pattern, causing the same invalid HTML. Refactor as suggested for the Linux block.
🧹 Nitpick comments (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
21-114: Consider de-duplicating the two almost-identical components to reduce maintenance overhead.
AdcsEsc14ScenarioALinuxandAdcsEsc14ScenarioAWindowsshare ~80 % of their content (license header, intro text, requirements list, cleanup steps). Divergence is limited to OS-specific commands. Extract the common narrative into a base component (or MDX/markdown blob) and inject platform-specific code snippets via props. This will:
- Cut file size nearly in half
- Ensure future edits (wording, links, requirements) stay in sync
- Reduce translation / localization work later
Not urgent for functionality, but worthwhile before this proliferates to other ESC scenarios.
Also applies to: 116-213
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
Learnt from: JonasBK
PR: #1671
File: packages/go/analysis/ad/esc16.go:65-69
Timestamp: 2025-07-10T14:31:01.781Z
Learning: In BloodHound ADCS ESC analysis functions (PostADCSESC1, PostADCSESC4, PostADCSESC6, PostADCSESC9, PostADCSESC10, PostADCSESC13, PostADCSESC16, etc.), the established pattern for handling cert template validation errors and user filtering errors is to use slog.WarnContext to log warnings and continue processing with the next template, rather than returning errors to the caller. This pattern is consistently applied across all ADCS ESC edge implementations.
🪛 Gitleaks (8.27.2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx
90-90: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
110-110: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
188-188: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
208-208: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-analysis
- GitHub Check: run-tests
- GitHub Check: build-ui
- GitHub Check: run-tests
...s/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
24-44: Nested block-level elements under<Typography>still generate invalid markup
Typographywithvariant="body2"renders a<p>. Embedding<p>,<ol>, and<ul>inside it creates illegal structures (<p><p>/<p><ol>…) and breaks semantics & screen-reader behaviour.
The same issue was raised on previous commits but persists in both the Linux (lines 35-44) and Windows (lines 132-141) variants. Please extract the lists/paragraphs into separateTypography component="div"wrappers or plain elements.Also applies to: 132-141
🧹 Nitpick comments (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
90-91: False-positive secret detection – consider adding allow-list commentStatic analysis (Gitleaks) flags the sample SHA-1 fingerprints in the
altSecurityIdentitiesstrings as “generic API keys”. They are obviously non-secret demo values, but the finding will keep re-appearing in CI.
Add a repo-appropriate suppression (e.g.# pragma: allowlist secret) or mask part of the hash to silence noise while keeping the example useful.Also applies to: 110-111, 188-189, 208-209
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx (1)
Learnt from: JonasBK
PR: #1671
File: packages/go/analysis/ad/esc16.go:65-69
Timestamp: 2025-07-10T14:31:01.781Z
Learning: In BloodHound ADCS ESC analysis functions (PostADCSESC1, PostADCSESC4, PostADCSESC6, PostADCSESC9, PostADCSESC10, PostADCSESC13, PostADCSESC16, etc.), the established pattern for handling cert template validation errors and user filtering errors is to use slog.WarnContext to log warnings and continue processing with the next template, rather than returning errors to the caller. This pattern is consistently applied across all ADCS ESC edge implementations.
🪛 Gitleaks (8.27.2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/AdcsEsc14ScenarioA/AdcsEsc14ScenarioA.tsx
90-90: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
110-110: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
188-188: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
208-208: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: build-ui
- GitHub Check: run-analysis
- GitHub Check: run-tests
- GitHub Check: run-tests
packages/javascript/bh-shared-ui/src/components/HelpTexts/WriteDacl/WindowsAbuse.tsx
Outdated
Show resolved
Hide resolved
JonasBK
added
the
blocked
Description
This PR adds support for ADCS ESC14 Scenario A attack paths by introducing new edge types and enhancing existing ones.
Motivation and Context
Resolves BED-6155
See above ticket for details.
Depends on this SharpHoundCommon PR: SpecterOps/SharpHoundCommon#235
How Has This Been Tested?
Locally with this dataset:
20250611101235_BloodHound.zip
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation