feat: Get-OffsetSignature (Authenticode provenance); strip long dashes - 3.3.0 - #21
Merged
Conversation
Contributor
Reviewer's GuideImplements a new Windows-only Authenticode provenance feature centered around the public Get-OffsetSignature cmdlet and its OffsetInspect.SignatureInfo output type, wires it into the module manifest/exports, adds unit and end-to-end tests for signature shaping, bumps the module to v3.3.0, and updates README/CHANGELOG/docs to document the new command and normalize dash usage. Sequence diagram for Get-OffsetSignature Authenticode verification flowsequenceDiagram
actor User
participant Get_OffsetSignature
participant Test_OIIsWindows
participant Get_AuthenticodeSignature
participant ConvertTo_OISignatureInfo
User->>Get_OffsetSignature: Get-OffsetSignature FilePath
Get_OffsetSignature->>Test_OIIsWindows: Test-OIIsWindows
Test_OIIsWindows-->>Get_OffsetSignature: isWindows
alt [not Windows]
Get_OffsetSignature-->>User: throw "requires Windows"
else [Windows]
Get_OffsetSignature->>Get_OffsetSignature: Resolve-Path / Get-Item
Get_OffsetSignature->>Get_AuthenticodeSignature: Get-AuthenticodeSignature LiteralPath
Get_AuthenticodeSignature-->>Get_OffsetSignature: Signature
Get_OffsetSignature->>ConvertTo_OISignatureInfo: ConvertTo-OISignatureInfo Signature File FileSize
ConvertTo_OISignatureInfo-->>Get_OffsetSignature: SignatureInfo
Get_OffsetSignature-->>User: OffsetInspect.SignatureInfo
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="module/OffsetInspect/Private/Threat.Signature.ps1" line_range="25-36" />
<code_context>
+ [CmdletBinding()]
+ param([AllowNull()][string]$DistinguishedName)
+ if ([string]::IsNullOrWhiteSpace($DistinguishedName)) { return $null }
+ foreach ($part in $DistinguishedName -split ',') {
+ $trimmed = $part.Trim()
+ if ($trimmed -match '^CN=(.+)$') { return $Matches[1].Trim() }
</code_context>
<issue_to_address>
**suggestion:** Distinguished name parsing may break on escaped commas or differently formatted DNs.
Splitting on a plain comma will misparse DNs with escaped commas (e.g. `CN=Example\, Inc.,O=Example`). If those are possible, use `System.Security.Cryptography.X509Certificates.X500DistinguishedName` (`Decode` with appropriate `X500DistinguishedNameFlags`) to extract CN, or at least use a DN-aware parsing approach instead of a raw `-split ','`.
```suggestion
function Get-OICommonName {
# Pull the CN (common name) out of an X.500 subject/issuer for a readable label, falling
# back to the full distinguished name.
[CmdletBinding()]
param([AllowNull()][string]$DistinguishedName)
if ([string]::IsNullOrWhiteSpace($DistinguishedName)) { return $null }
# Prefer DN-aware parsing via X500DistinguishedName to handle escaped commas and formatting
try {
$dnObject = [System.Security.Cryptography.X509Certificates.X500DistinguishedName]::new($DistinguishedName)
$decoded = $dnObject.Decode(
[System.Security.Cryptography.X509Certificates.X500DistinguishedNameFlags]::UseNewLines
)
if (-not [string]::IsNullOrWhiteSpace($decoded)) {
foreach ($line in $decoded -split "`n") {
$trimmed = $line.Trim()
if ($trimmed -match '^CN=(.+)$') {
return $Matches[1].Trim()
}
}
}
}
catch {
# Fall through to the simple fallback below on invalid or unparsable DNs
}
# Fallback: if we couldn't parse via X500DistinguishedName, return the original DN trimmed
return $DistinguishedName.Trim()
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+25
to
+36
| function Get-OICommonName { | ||
| # Pull the CN (common name) out of an X.500 subject/issuer for a readable label, falling | ||
| # back to the full distinguished name. | ||
| [CmdletBinding()] | ||
| param([AllowNull()][string]$DistinguishedName) | ||
| if ([string]::IsNullOrWhiteSpace($DistinguishedName)) { return $null } | ||
| foreach ($part in $DistinguishedName -split ',') { | ||
| $trimmed = $part.Trim() | ||
| if ($trimmed -match '^CN=(.+)$') { return $Matches[1].Trim() } | ||
| } | ||
| return $DistinguishedName.Trim() | ||
| } |
Contributor
There was a problem hiding this comment.
suggestion: Distinguished name parsing may break on escaped commas or differently formatted DNs.
Splitting on a plain comma will misparse DNs with escaped commas (e.g. CN=Example\, Inc.,O=Example). If those are possible, use System.Security.Cryptography.X509Certificates.X500DistinguishedName (Decode with appropriate X500DistinguishedNameFlags) to extract CN, or at least use a DN-aware parsing approach instead of a raw -split ','.
Suggested change
| function Get-OICommonName { | |
| # Pull the CN (common name) out of an X.500 subject/issuer for a readable label, falling | |
| # back to the full distinguished name. | |
| [CmdletBinding()] | |
| param([AllowNull()][string]$DistinguishedName) | |
| if ([string]::IsNullOrWhiteSpace($DistinguishedName)) { return $null } | |
| foreach ($part in $DistinguishedName -split ',') { | |
| $trimmed = $part.Trim() | |
| if ($trimmed -match '^CN=(.+)$') { return $Matches[1].Trim() } | |
| } | |
| return $DistinguishedName.Trim() | |
| } | |
| function Get-OICommonName { | |
| # Pull the CN (common name) out of an X.500 subject/issuer for a readable label, falling | |
| # back to the full distinguished name. | |
| [CmdletBinding()] | |
| param([AllowNull()][string]$DistinguishedName) | |
| if ([string]::IsNullOrWhiteSpace($DistinguishedName)) { return $null } | |
| # Prefer DN-aware parsing via X500DistinguishedName to handle escaped commas and formatting | |
| try { | |
| $dnObject = [System.Security.Cryptography.X509Certificates.X500DistinguishedName]::new($DistinguishedName) | |
| $decoded = $dnObject.Decode( | |
| [System.Security.Cryptography.X509Certificates.X500DistinguishedNameFlags]::UseNewLines | |
| ) | |
| if (-not [string]::IsNullOrWhiteSpace($decoded)) { | |
| foreach ($line in $decoded -split "`n") { | |
| $trimmed = $line.Trim() | |
| if ($trimmed -match '^CN=(.+)$') { | |
| return $Matches[1].Trim() | |
| } | |
| } | |
| } | |
| } | |
| catch { | |
| # Fall through to the simple fallback below on invalid or unparsable DNs | |
| } | |
| # Fallback: if we couldn't parse via X500DistinguishedName, return the original DN trimmed | |
| return $DistinguishedName.Trim() | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add Authenticode provenance verification to OffsetInspect and release it as version 3.3.0.
New Features:
Enhancements:
Documentation:
Tests: