[EDR Workflows] Add Signer option to Mac trusted apps

[szwarckonrad]

This PR adds a Signer condition for trusted apps on macOS. Previously, users could only build conditions using hash, path, and signer options on Windows. With these changes, macOS also supports the Signer option, leaving only Linux limited to Path and Hash options.

Screen.Recording.2024-10-28.at.10.16.05.mov

[elasticmachine]

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

[ashokaditya]

Did also you verify that the signer entries made for mac os are correctly generated in the endpoint artifact?

[szwarckonrad]

Did also you verify that the signer entries made for mac os are correctly generated in the endpoint artifact?

@ashokaditya
I did observe the artifact id change. Could you help me out with a way to validate it further?

We do plan on testing it E2E with an agent installed on MacOS though ;)

[gergoabraham]

looks and works great! 🚀 (not e2e, just UI test and looked at SOs)
there's only some nitpicking

[gergoabraham]

thanks for the changes! 🚀

[ashokaditya]

Did also you verify that the signer entries made for mac os are correctly generated in the endpoint artifact?

@ashokaditya I did observe the artifact id change. Could you help me out with a way to validate it further?

We do plan on testing it E2E with an agent installed on MacOS though ;)

You can use the artifact downloader to test that the entries are correctly generated.

[gergoabraham]

thanks for the changes, @szwarckonrad!

there's a small hiccup with the form when changing OS when windows/macOS and Signature is selected:

• if you switch to Linux, the field defaults back to Hash,
• but if you switch between win/mac, the field simply goes blank

also, when you try to save it, it shows that API input validation works perfect! 😄 🚀

Screen.Recording.2024-11-05.at.11.27.39.mov

[gergoabraham]

i've been thinking about this change. i can't put my finger on it why there is a type error without this change, but it feels broken. i mean - we cannot assign an array of TrustedAppConditionEntry type to NewTrustedApp['entries']? it just doesn't seem right

i'll think about this a bit more. do you have any idea?

[gergoabraham]

looks like the problem roots around using enums as generics, where we want to create a type where only a partial set of the enum elements are should be used

when we create TrustedAppConditionEntry, we want its field: T to possibly have the values ot the ConditionEntryField enum, but in cases, only a subset of those (e.g. Linux only has path and hash)

export interface TrustedAppConditionEntry<T extends ConditionEntryField = ConditionEntryField>

but, in the test, we fall back to the default generic type, ConditionEntryField, that has all enum elements

so far it has worked, because accidentally one of the derived types contained all elements of the enum:

export type WindowsConditionEntry = TrustedAppConditionEntry<
  ConditionEntryField.HASH | ConditionEntryField.PATH | ConditionEntryField.SIGNER
>;

which ensured that there is a possibility, using the default of the generic (i.e. ConditionEntryField), to meet this criteria

but, with your modification, now neither of the derived types contains all of the enum elements, hence the error message. and i have no idea how to fix it either in the generic rule, or defining the generics type in the test file

still, looks like there's a nice quick fix to avoid the type mess: type assertion! i'll add it in a new comment

---

btw, i was worried about this issue, bc type system is one of our guard on code quality, and although automated and manual tests showed that everything works fine, still didn't want to throw out the typing

[gergoabraham]

couldn't add it in a suggestion, bc the lines are out of your modifications, so here it is simply:

    const createNewTrustedApp = <T>(data?: T): NewTrustedApp =>
      ({
        name: 'Some Anti-Virus App',
        description: 'this one is ok',
        os: OperatingSystem.WINDOWS,
        effectScope: { type: 'global' },
        entries: [createConditionEntry()],
        ...(data || {}),
      } as NewTrustedApp);

simply asserting the created object as NewTrustedApp solves the type error, while ensures that the created object aligns with the expectations in both places

[szwarckonrad]

Makes total sense! Thanks for looking into this, I appreciate it! 9d39376

[vitaliidm]

Detection Engine LGTM

There are changes not directly used in our part of app, but triggered review request, due to entire utils folder belonging to the team.
Which we'll revise on team sync, what could be the best approach here

packages/kbn-securitysolution-utils @elastic/security-detection-engine

[gergoabraham]

thanks for the fixes, i confirm that switching between windows and mac works perfect now! 👍

i'll be happy if you fix the type issue, but independently from that, nice work! 🚀

#197821 (comment)

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: c47230d
• Cloud Deployment

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
securitySolution	13.4MB	13.4MB	+865.0B

Unknown metric groups

References to deprecated APIs

id	before	after	diff
securitySolution	448	438	-10

History

• 💚 Build #250828 succeeded 9d39376
• 💚 Build #250203 succeeded e9515ac
• 💔 Build #248470 failed cdbdc0a
• 💚 Build #248387 succeeded 3b4a557
• 💚 Build #248107 succeeded 19cfd4f

cc @szwarckonrad

[kibanamachine]

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11893336148

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation