Skip to content

Implement Named Constraints#288

Merged
Entenwilli merged 2 commits intomainfrom
dsl-constraint-names
Jun 4, 2025
Merged

Implement Named Constraints#288
Entenwilli merged 2 commits intomainfrom
dsl-constraint-names

Conversation

@Entenwilli
Copy link
Copy Markdown
Member

@Entenwilli Entenwilli commented May 28, 2025

This PR implements named constraints, so they can be displayed to the user in a more intuitive manner.

However, there are still some discussion points (please provide your opinion on the following, @01Parzival10 @sebinside @Nicolas-Boltz):

  • Allow naming a constraint directly in the DSL: An simple example DataOwnership: data Sensitivity.Personal neverFlows Location.nonEU would create a constraint named DataOwnership
  • Should we ask for a constraint name in the CLI (or just use the provided names, when above exists)?

@Entenwilli Entenwilli added this to the 5.0.0 milestone May 28, 2025
@Entenwilli Entenwilli self-assigned this May 28, 2025
@Entenwilli Entenwilli added enhancement New feature or request core Related to the core DFD/PCM data flow analysis labels May 28, 2025
@Entenwilli Entenwilli force-pushed the dsl-constraint-names branch from 73398e2 to 6a550a6 Compare May 28, 2025 06:45
Copy link
Copy Markdown
Contributor

@01Parzival10 01Parzival10 left a comment

Choose a reason for hiding this comment

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

It's important that the name can be accessed in DSLResult. Without a link between violation and constraint, batch processing becomes tedious.

@sebinside
Copy link
Copy Markdown
Member

@Nicolas-Boltz and I discussed the implications of changing the format and we have the following proposal:

  • A constraint (does not matter whether text file via CLI or web editor) starts with a bullet point *, followed by an optional space and the name of the constraint which may be any string until the delimiter :
  • Afterwards, the constraint logic is written down as is
  • The next constraint starts again with the bullet point *

By doing this, we both unify the formats but also require to we always name constraints (which significantly helps the understandability a couple of years later) and we also simplify the web editor. The constraints window than simply shows a multiline text editor without the need to manually add constraint entries (which is bad UI, as discussed in our last meeting).

Example for a valid constraint:

* DataOwnership: 
    data Sensitivity.Personal 
      neverFlows 
    vertex Location.nonEU
* SomethingCompletelyDifferent: data Sensitivity.Special neverFlows vertex Location.nonSpecial

@Entenwilli
Copy link
Copy Markdown
Member Author

It's important that the name can be accessed in DSLResult. Without a link between violation and constraint, batch processing becomes tedious.

As a DSLResult only contains violations for one TFG and is constructed by an AnalysisConstraint it should be enough to be able to access the name from there, right?

@Entenwilli Entenwilli requested a review from 01Parzival10 May 30, 2025 06:48
@Entenwilli
Copy link
Copy Markdown
Member Author

@Nicolas-Boltz and I discussed the implications of changing the format and we have the following proposal:

* A constraint (does not matter whether text file via CLI or web editor) starts with a bullet point `*`, followed by an optional space and the name of the constraint which may be any string until the delimiter `:`

* Afterwards, the constraint logic is written down as is

* The next constraint starts again with the bullet point `*`

By doing this, we both unify the formats but also require to we always name constraints (which significantly helps the understandability a couple of years later) and we also simplify the web editor. The constraints window than simply shows a multiline text editor without the need to manually add constraint entries (which is bad UI, as discussed in our last meeting).

Example for a valid constraint:

* DataOwnership: 
    data Sensitivity.Personal 
      neverFlows 
    vertex Location.nonEU
* SomethingCompletelyDifferent: data Sensitivity.Special neverFlows vertex Location.nonSpecial

Done in 057224b. I however would opt for using - instead of * for listing constraints, as that is more common in Markdown, and other plain text based file formats

@sebinside
Copy link
Copy Markdown
Member

Thank you. We intentionally choose the asterisk as it is less common in names (especially compared to the dash used to combine words), and additionally, the asterisk is already reserved as wildcard. However, allowing both would also be fine.

@Entenwilli
Copy link
Copy Markdown
Member Author

I would implement both, but just as a note (and consideration):
After reading the List Token (e.g. - or *) and an optional Space, we allow all characters until we encounter our name delimiter (so :). This would allow users to define any names they like, even ones with spaces or other (normally) reserved characters, like * or -.
Therefore, we would not have any conflicts when using -.

Considering this, would we still allow both?

@01Parzival10
Copy link
Copy Markdown
Contributor

It's important that the name can be accessed in DSLResult. Without a link between violation and constraint, batch processing becomes tedious.

As a DSLResult only contains violations for one TFG and is constructed by an AnalysisConstraint it should be enough to be able to access the name from there, right?

I could build around it. But for the sake of traceability I'd prefer a link between Result and Constraint

Copy link
Copy Markdown
Contributor

@01Parzival10 01Parzival10 left a comment

Choose a reason for hiding this comment

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

LGTM

@Entenwilli Entenwilli force-pushed the dsl-constraint-names branch from 057224b to 6fa9efb Compare June 4, 2025 13:16
@Entenwilli Entenwilli merged commit b330ae2 into main Jun 4, 2025
1 check passed
@Entenwilli Entenwilli deleted the dsl-constraint-names branch June 4, 2025 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Related to the core DFD/PCM data flow analysis enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants