From c799ec6707b2132582194ac3019b6a3f7c40e381 Mon Sep 17 00:00:00 2001 From: Mario Haefs Date: Wed, 5 Aug 2026 15:18:21 +0200 Subject: [PATCH 01/12] Enhance layout and accessibility styles in app component --- frontend/src/app/app.component.scss | 8 +++++++- frontend/src/styles.scss | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/frontend/src/app/app.component.scss b/frontend/src/app/app.component.scss index 80c664d..2774d49 100644 --- a/frontend/src/app/app.component.scss +++ b/frontend/src/app/app.component.scss @@ -1,5 +1,11 @@ +:host { + display: flex; + flex-direction: column; + min-height: 100vh; +} + .app-content { - min-height: calc(100vh - 64px); + flex: 1; // Programmatic focus target for skip link / route changes — the landmark // itself is not an interactive control, so no visible ring is needed. diff --git a/frontend/src/styles.scss b/frontend/src/styles.scss index 92a88e7..c45242b 100644 --- a/frontend/src/styles.scss +++ b/frontend/src/styles.scss @@ -1,5 +1,8 @@ @use './material'; @use 'mixins' as *; +// CDK's `.cdk-visually-hidden` class (used by LiveAnnouncer and others to +// hide live-region text visually while keeping it available to screen readers). +@use '@angular/cdk/a11y-prebuilt.css'; // ============================================================ // Construct-X Design Tokens From b0bb8beef10aecf4f2a139f56864d0e58ff989f6 Mon Sep 17 00:00:00 2001 From: Mario Haefs Date: Thu, 6 Aug 2026 11:39:07 +0200 Subject: [PATCH 02/12] chore(deps): update Angular packages to 21.2.19 Closes the two advisories reported by `npm audit --omit=dev` for 21.2.18: - GHSA-jj27-h5hq-8x99 (XSS via i18n event-handler attributes) - GHSA-jhpw-976m-542j (HttpTransferCache cross-request response reuse) Neither is exploitable here (the app uses Transloco instead of Angular i18n and has no SSR/hydration), but the fix is a patch release and `npm audit --omit=dev` now reports 0 vulnerabilities instead of 7 high. The ranges are raised from ^21.2.0 to ^21.2.19 so the patched version is the documented floor. The @angular/* lockfile entries had to be re-resolved as a group: npm anchors peer resolution on the installed tree, so updating them one by one deadlocks on `peerOptional @angular/animations@21.2.18 from @angular/platform-browser@21.2.18`. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/package-lock.json | 90 +++++++++++++++++++------------------- frontend/package.json | 16 +++---- 2 files changed, 53 insertions(+), 53 deletions(-) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index fcfb001..75ba640 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -8,15 +8,15 @@ "name": "frontend", "version": "0.0.0", "dependencies": { - "@angular/animations": "^21.2.0", + "@angular/animations": "^21.2.19", "@angular/cdk": "^21.2.2", - "@angular/common": "^21.2.0", - "@angular/compiler": "^21.2.0", - "@angular/core": "^21.2.0", - "@angular/forms": "^21.2.0", + "@angular/common": "^21.2.19", + "@angular/compiler": "^21.2.19", + "@angular/core": "^21.2.19", + "@angular/forms": "^21.2.19", "@angular/material": "^21.2.2", - "@angular/platform-browser": "^21.2.0", - "@angular/router": "^21.2.0", + "@angular/platform-browser": "^21.2.19", + "@angular/router": "^21.2.19", "@jsverse/transloco": "^8.3.0", "rxjs": "~7.8.0", "tslib": "^2.3.0" @@ -24,7 +24,7 @@ "devDependencies": { "@angular/build": "^21.2.2", "@angular/cli": "^21.2.2", - "@angular/compiler-cli": "^21.2.0", + "@angular/compiler-cli": "^21.2.19", "@eslint/js": "^10.0.1", "@vitest/coverage-v8": "^4.1.10", "angular-eslint": "21.3.0", @@ -438,9 +438,9 @@ } }, "node_modules/@angular/animations": { - "version": "21.2.18", - "resolved": "https://registry.npmjs.org/@angular/animations/-/animations-21.2.18.tgz", - "integrity": "sha512-zN++Qb4Oz4x/5LYHuW9FItm7aHsV4HBz518GT25QusMvGymh+4io2TWhPYB0C9jx+g5q8GeQLd3c8r42yLrRxw==", + "version": "21.2.19", + "resolved": "https://registry.npmjs.org/@angular/animations/-/animations-21.2.19.tgz", + "integrity": "sha512-qC0mviselvpKq2ID9bT+USnCsHKfwZAifkUr5A4h/WCzXvUvpVNWp/RPhGcKeOR+seg3EO5GtIZyblwiRlhzrA==", "deprecated": "@angular/animations is deprecated. Use `animate.enter` and `animate.leave` instead. For more information see: https://v22.angular.dev/guide/animations.", "license": "MIT", "dependencies": { @@ -450,7 +450,7 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/core": "21.2.18" + "@angular/core": "21.2.19" } }, "node_modules/@angular/build": { @@ -605,9 +605,9 @@ } }, "node_modules/@angular/common": { - "version": "21.2.18", - "resolved": "https://registry.npmjs.org/@angular/common/-/common-21.2.18.tgz", - "integrity": "sha512-gZugZ8gX/KkACIZ3/ekoImLu5z8a0Iu9O5b84kh8e+++VUjmkmd19IygBsq/8/fF3vKf0UcIKcx3P6A/gb2DJw==", + "version": "21.2.19", + "resolved": "https://registry.npmjs.org/@angular/common/-/common-21.2.19.tgz", + "integrity": "sha512-Rvo/VXI0kUmfQT7+OeAjv526OJlf/WrLnJq1Tz84Jkyv9bs9SOMTCT6m4+boo8gxVNdrcxvGU3Q0o2jZzKceSg==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -616,14 +616,14 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/core": "21.2.18", + "@angular/core": "21.2.19", "rxjs": "^6.5.3 || ^7.4.0" } }, "node_modules/@angular/compiler": { - "version": "21.2.18", - "resolved": "https://registry.npmjs.org/@angular/compiler/-/compiler-21.2.18.tgz", - "integrity": "sha512-ccnDuKLuzIa0ayijR+alarsHNWIksuGV/lxGTZ0t6/0+B6J/RXupz6M2IO6ZHHEcg8r7pcrLCUBTyp3FoiRJrg==", + "version": "21.2.19", + "resolved": "https://registry.npmjs.org/@angular/compiler/-/compiler-21.2.19.tgz", + "integrity": "sha512-vuF5i1t14ftJiHXVVLgDYLWkT99QuajphcUHy1VZaMX3FiqSGXRV62g+R2RMxL0OJ5C1ai8xTHKPx9n1lQFyFg==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -633,9 +633,9 @@ } }, "node_modules/@angular/compiler-cli": { - "version": "21.2.18", - "resolved": "https://registry.npmjs.org/@angular/compiler-cli/-/compiler-cli-21.2.18.tgz", - "integrity": "sha512-L5zbIp7YfTTB4I4xT33FgEBanzhppNejzZX0HJOEqKDyDL2jXq+flt83lE0XVuRJ6/zrG+UKj0B+y/CK6Ro7Wg==", + "version": "21.2.19", + "resolved": "https://registry.npmjs.org/@angular/compiler-cli/-/compiler-cli-21.2.19.tgz", + "integrity": "sha512-QxWqUvhTWgyYPPkfpupOUIEa3Y5cbzMilaFgRNpkvFy6teARw5Izsd7RxUbj3Tp3xJOi1MtumNmkxLxmv3iv7A==", "dev": true, "license": "MIT", "dependencies": { @@ -656,7 +656,7 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/compiler": "21.2.18", + "@angular/compiler": "21.2.19", "typescript": ">=5.9 <6.1" }, "peerDependenciesMeta": { @@ -666,9 +666,9 @@ } }, "node_modules/@angular/core": { - "version": "21.2.18", - "resolved": "https://registry.npmjs.org/@angular/core/-/core-21.2.18.tgz", - "integrity": "sha512-AG4bb6GU0+qp+vVBjc/IhSPjgcRX8QsCb8jzN8dDyhnjsyuuG/LlIiU0l6n65BI9SGKE9m6TfsJ1ObkkAiE5KQ==", + "version": "21.2.19", + "resolved": "https://registry.npmjs.org/@angular/core/-/core-21.2.19.tgz", + "integrity": "sha512-PVoXD1kBexOJLkFzKx2zBY/0oZJXGru0eGn2hu0q5n3vkZlYsR1yRolfGenK1gZE48Qibiw/ttg/X/pAIPEZGg==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -677,7 +677,7 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/compiler": "21.2.18", + "@angular/compiler": "21.2.19", "rxjs": "^6.5.3 || ^7.4.0", "zone.js": "~0.15.0 || ~0.16.0" }, @@ -691,9 +691,9 @@ } }, "node_modules/@angular/forms": { - "version": "21.2.18", - "resolved": "https://registry.npmjs.org/@angular/forms/-/forms-21.2.18.tgz", - "integrity": "sha512-TrRuiNjIzrrNtQgpJVH5gultQrvBlawa+tTzIpxBfIqLpkHrTPZLtYu118EUk6jhWzgRhKYUorInjJe+sSxC+w==", + "version": "21.2.19", + "resolved": "https://registry.npmjs.org/@angular/forms/-/forms-21.2.19.tgz", + "integrity": "sha512-tEw8cz2UU6VSB+ReJN86k87nRdLRXGi+8SZhoRl2dFu2iReIO3S6kJAVTH7Y9kdrokqTPhWG+KNEzWgqhdjXOg==", "license": "MIT", "dependencies": { "@standard-schema/spec": "^1.0.0", @@ -703,9 +703,9 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/common": "21.2.18", - "@angular/core": "21.2.18", - "@angular/platform-browser": "21.2.18", + "@angular/common": "21.2.19", + "@angular/core": "21.2.19", + "@angular/platform-browser": "21.2.19", "rxjs": "^6.5.3 || ^7.4.0" } }, @@ -727,9 +727,9 @@ } }, "node_modules/@angular/platform-browser": { - "version": "21.2.18", - "resolved": "https://registry.npmjs.org/@angular/platform-browser/-/platform-browser-21.2.18.tgz", - "integrity": "sha512-zltF+3HrlgtZbYg8U98LhG0dyGm1aHT3Px8//9wjqi/TUY8UlHsYKByL197QtVWDBjf/dekzXdz5c+iyVtanLQ==", + "version": "21.2.19", + "resolved": "https://registry.npmjs.org/@angular/platform-browser/-/platform-browser-21.2.19.tgz", + "integrity": "sha512-YMguYVhdkV8tr9MvbN+VpMjbdmsYq6g12M9WPvAYM51BkL2iREbWeoXDGmfCw9G0it6+0pMdgrSPFFUFuOqpAQ==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -738,9 +738,9 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/animations": "21.2.18", - "@angular/common": "21.2.18", - "@angular/core": "21.2.18" + "@angular/animations": "21.2.19", + "@angular/common": "21.2.19", + "@angular/core": "21.2.19" }, "peerDependenciesMeta": { "@angular/animations": { @@ -749,9 +749,9 @@ } }, "node_modules/@angular/router": { - "version": "21.2.18", - "resolved": "https://registry.npmjs.org/@angular/router/-/router-21.2.18.tgz", - "integrity": "sha512-Xix19uG1YthC8IslhWKSfPdhscVWREBGVJZW2OnRmLef8F7DvhF49S6vOywaBo+Uahe0egkmgWDNpEwxAzsgLw==", + "version": "21.2.19", + "resolved": "https://registry.npmjs.org/@angular/router/-/router-21.2.19.tgz", + "integrity": "sha512-cKO/aq1xEMvgS29jFUc/Y3KsgEzHnwOw6sEL+UQZkZTmGD5YrYv8Yvj05GDMEUYbvDxBd5DixVVEbH38lyQKqA==", "license": "MIT", "dependencies": { "tslib": "^2.3.0" @@ -760,9 +760,9 @@ "node": "^20.19.0 || ^22.12.0 || >=24.0.0" }, "peerDependencies": { - "@angular/common": "21.2.18", - "@angular/core": "21.2.18", - "@angular/platform-browser": "21.2.18", + "@angular/common": "21.2.19", + "@angular/core": "21.2.19", + "@angular/platform-browser": "21.2.19", "rxjs": "^6.5.3 || ^7.4.0" } }, diff --git a/frontend/package.json b/frontend/package.json index ed3b035..97bbbb1 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -22,15 +22,15 @@ "private": true, "packageManager": "npm@11.0.0", "dependencies": { - "@angular/animations": "^21.2.0", + "@angular/animations": "^21.2.19", "@angular/cdk": "^21.2.2", - "@angular/common": "^21.2.0", - "@angular/compiler": "^21.2.0", - "@angular/core": "^21.2.0", - "@angular/forms": "^21.2.0", + "@angular/common": "^21.2.19", + "@angular/compiler": "^21.2.19", + "@angular/core": "^21.2.19", + "@angular/forms": "^21.2.19", "@angular/material": "^21.2.2", - "@angular/platform-browser": "^21.2.0", - "@angular/router": "^21.2.0", + "@angular/platform-browser": "^21.2.19", + "@angular/router": "^21.2.19", "@jsverse/transloco": "^8.3.0", "rxjs": "~7.8.0", "tslib": "^2.3.0" @@ -38,7 +38,7 @@ "devDependencies": { "@angular/build": "^21.2.2", "@angular/cli": "^21.2.2", - "@angular/compiler-cli": "^21.2.0", + "@angular/compiler-cli": "^21.2.19", "@eslint/js": "^10.0.1", "@vitest/coverage-v8": "^4.1.10", "angular-eslint": "21.3.0", From 1926104b7f649278c225aa6568a0f34c79bd7112 Mon Sep 17 00:00:00 2001 From: Mario Haefs Date: Thu, 6 Aug 2026 11:54:37 +0200 Subject: [PATCH 03/12] fix(policy-builder): prevent translation-parameter injection in legal text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Transloco re-scans the result of a placeholder substitution for further placeholders: DefaultTranspiler.transpile() loops over the already substituted string and `interpolationMatcher` is a getter returning a fresh RegExp with lastIndex = 0 on every access. A parameter value that itself contains `{{...}}` is therefore resolved a second time. Reproduced against the installed Transloco 8.4.0: agreement = "{{legalDescription.unrestricted}}" -> splices an unrelated sentence into the legally binding text agreement = "{{constructor}}" -> "function Object() { [native code] }" agreement = "{{agreement}}" -> substitutes itself, the string never changes, the while loop never terminates and the browser tab freezes Constraint values are not only produced by the UI (where a dropdown and a constant cap them) but also read from GET /v1/policies/:id. An HTTP response is input; the Constraint type is only a compile-time promise. Fixed by only forwarding values that provably come from the metadata registry: `agreement` is compared against FRAMEWORK_AGREEMENT_VALUE, use-case labels are looked up in USE_CASE_OPTIONS instead of building the i18n key from the id, and formatDate() no longer falls back to the raw value. Anything else renders as "—". Tests assert the invariant directly (no `{{` or `}}` in any key or parameter handed to translate) and fail without the fix. Co-Authored-By: Claude Opus 5 (1M context) --- .../helpers/legal-description.helper.spec.ts | 93 +++++++++++++++++-- .../helpers/legal-description.helper.ts | 49 ++++++++-- 2 files changed, 129 insertions(+), 13 deletions(-) diff --git a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.spec.ts b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.spec.ts index bd24476..da5d91f 100644 --- a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.spec.ts +++ b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.spec.ts @@ -1,8 +1,9 @@ -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, vi, type MockedFunction } from 'vitest'; import { TranslocoService } from '@jsverse/transloco'; import { Constraint } from '@shared/types/constraint.model'; import { Policy } from '@shared/types/policy.model'; +import { FRAMEWORK_AGREEMENT_VALUE } from '@features/policies/builder/metadata/use-case-options.data'; import { buildLegalClauses, buildLegalDescription } from './legal-description.helper'; @@ -85,12 +86,15 @@ describe('buildLegalDescription', () => { }); describe('Use-Case-Liste (joinList)', () => { + // Nur IDs aus USE_CASE_OPTIONS verwenden: unbekannte IDs werden bewusst zu "—" + // zusammengefaltet (siehe "Schutz vor Transloco-Parameter-Injection") und würden + // die Trennlogik hier nicht mehr sichtbar machen. function useCaseList(useCases: string[]): string { const text = buildLegalDescription( draft('ACCESS', [{ type: 'USE_CASE', useCases }]), makeTransloco(), ); - // Übersetzte Labels sind hier `useCase.`. + // Übersetzte Labels sind hier der i18n-Key aus der Registry, also `useCase.`. const match = /legalDescription\.clause\.useCase\[list=(.+?)\]/.exec(text); return match![1]; } @@ -100,11 +104,15 @@ describe('buildLegalDescription', () => { }); it('zwei Use-Cases → mit "&" verbunden', () => { - expect(useCaseList(['UC.geodata', 'UC.quality'])).toBe('useCase.geodata & useCase.quality'); + expect(useCaseList(['UC.geodata', 'UC.quality-assurance'])).toBe( + 'useCase.geodata & useCase.quality-assurance', + ); }); it('drei Use-Cases → Komma-getrennt, letztes mit "&"', () => { - expect(useCaseList(['UC.a', 'UC.b', 'UC.c'])).toBe('useCase.a, useCase.b & useCase.c'); + expect(useCaseList(['UC.geodata', 'UC.material-testing', 'UC.bim-coordination'])).toBe( + 'useCase.geodata, useCase.material-testing & useCase.bim-coordination', + ); }); }); @@ -200,12 +208,12 @@ describe('buildLegalDescription', () => { expect(text).toContain('start=—;end=—'); }); - it('unparsebares Datum → unverändert durchgereicht', () => { + it('unparsebares Datum → Platzhalter "—" statt Rohwert', () => { const text = buildLegalDescription( draft('CONTRACT', [{ type: 'DATE_RANGE', startDate: 'not-a-date', endDate: 'not-a-date' }]), makeTransloco(), ); - expect(text).toContain('start=not-a-date;end=not-a-date'); + expect(text).toContain('start=—;end=—'); }); }); @@ -217,4 +225,77 @@ describe('buildLegalDescription', () => { expect(t.translate).toHaveBeenCalledWith('constraint.MEMBERSHIP.legalText', undefined, 'de'); }); }); + + /** + * Transloco durchsucht das Ergebnis einer Ersetzung ERNEUT nach Platzhaltern: + * `DefaultTranspiler.transpile()` läuft in einer `while`-Schleife über den bereits + * ersetzten String, und `interpolationMatcher` ist ein Getter, der jedes Mal ein + * frisches RegExp mit `lastIndex = 0` liefert. Ein Parameterwert, der selbst + * `{{…}}` enthält, wird dadurch ein zweites Mal aufgelöst — im Fall + * `{{agreement}}` → `{{agreement}}` ändert sich der String nie und die Schleife + * terminiert nicht (eingefrorener Browser-Tab). + * + * Constraint-Werte stammen nicht nur aus der UI, sondern auch aus + * `GET /v1/policies/:id`. Deshalb die Invariante: an `translate()` darf weder als + * Key noch als Parameterwert jemals ein Interpolations-Delimiter gelangen. + */ + describe('Schutz vor Transloco-Parameter-Injection', () => { + const HOSTILE = '{{agreement}}'; + + function argumentsPassedTo(t: TranslocoService): string[] { + const calls = (t.translate as unknown as MockedFunction).mock + .calls; + return calls.flatMap(([key, params]) => [ + String(key), + ...(params ? Object.values(params).map((v) => String(v)) : []), + ]); + } + + const hostileConstraints: [string, Constraint][] = [ + ['FRAMEWORK_AGREEMENT.agreement', { type: 'FRAMEWORK_AGREEMENT', agreement: HOSTILE }], + ['USE_CASE.useCases', { type: 'USE_CASE', useCases: [HOSTILE] }], + ['DATE_RANGE.startDate', { type: 'DATE_RANGE', startDate: HOSTILE, endDate: '2027-01-01' }], + ['DATE_RANGE.endDate', { type: 'DATE_RANGE', startDate: '2027-01-01', endDate: HOSTILE }], + ]; + + for (const [label, constraint] of hostileConstraints) { + it(`reicht keinen Interpolations-Delimiter an Transloco weiter: ${label}`, () => { + const t = makeTransloco(); + buildLegalDescription(draft('CONTRACT', [constraint]), t, 'de'); + + for (const arg of argumentsPassedTo(t)) { + expect(arg).not.toContain('{{'); + expect(arg).not.toContain('}}'); + } + }); + } + + it('ersetzt einen unbekannten Rahmenvertrag durch den Platzhalter "—"', () => { + const text = buildLegalDescription( + draft('CONTRACT', [{ type: 'FRAMEWORK_AGREEMENT', agreement: 'FremderVertrag' }]), + makeTransloco(), + ); + expect(text).toContain('agreement=—'); + }); + + it('ersetzt einen unbekannten Use-Case durch den Platzhalter "—"', () => { + const text = buildLegalDescription( + draft('CONTRACT', [{ type: 'USE_CASE', useCases: ['UC.gibt-es-nicht'] }]), + makeTransloco(), + ); + expect(text).toContain('list=—'); + }); + + it('lässt bekannte Werte unverändert', () => { + const text = buildLegalDescription( + draft('CONTRACT', [ + { type: 'FRAMEWORK_AGREEMENT', agreement: FRAMEWORK_AGREEMENT_VALUE }, + { type: 'USE_CASE', useCases: ['UC.quality-assurance', 'UC.geodata'] }, + ]), + makeTransloco(), + ); + expect(text).toContain(`agreement=${FRAMEWORK_AGREEMENT_VALUE}`); + expect(text).toContain('list=useCase.quality-assurance & useCase.geodata'); + }); + }); }); diff --git a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.ts b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.ts index 06a0554..c2be1c7 100644 --- a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.ts +++ b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.ts @@ -2,6 +2,16 @@ import { TranslocoService } from '@jsverse/transloco'; import { Constraint } from '@shared/types/constraint.model'; import { Policy } from '@shared/types/policy.model'; import { CONSTRAINT_METADATA } from '@features/policies/builder/metadata/constraint-metadata'; +import { + FRAMEWORK_AGREEMENT_VALUE, + USE_CASE_OPTIONS, +} from '@features/policies/builder/metadata/use-case-options.data'; + +/** + * Anzeigewert für Constraint-Inhalte, die nicht aus der Metadaten-Registry stammen — + * etwa weil das Backend einen unbekannten Use-Case oder Rahmenvertrag geliefert hat. + */ +const UNKNOWN_VALUE = '—'; export interface LegalClause { /** Anzeigename des Constraints (Metadaten-Label), dient als Überschrift des Unterpunkts. */ @@ -74,16 +84,28 @@ export function buildLegalDescription( return `${intro}\n\n${list}`; } +/** + * WICHTIG — warum hier gegen Whitelists geprüft wird statt die Werte direkt zu verwenden: + * + * Transloco durchsucht das Ergebnis einer Platzhalter-Ersetzung ERNEUT nach Platzhaltern + * (`DefaultTranspiler.transpile()` iteriert über den bereits ersetzten String, und + * `interpolationMatcher` liefert bei jedem Zugriff ein frisches RegExp mit `lastIndex = 0`). + * Ein Parameterwert, der selbst `{{…}}` enthält, wird dadurch ein zweites Mal aufgelöst: + * `{{legalDescription.unrestricted}}` schiebt einen fremden Satz in den Rechtstext, und + * `{{agreement}}` ersetzt sich endlos selbst — die Schleife terminiert nie und der Tab friert ein. + * + * Constraint-Werte stammen nicht nur aus der UI (dort sind sie durch Dropdown bzw. Konstante + * gedeckelt), sondern auch aus `GET /v1/policies/:id`. Ein HTTP-Response ist Eingabe; der + * `Constraint`-Typ ist nur ein Compile-Zeit-Versprechen. Deshalb: nur Werte weiterreichen, + * die nachweislich aus der Metadaten-Registry stammen. + */ function buildClause(c: Constraint, transloco: TranslocoService, lang?: string): string { const meta = CONSTRAINT_METADATA[c.type]; const base = transloco.translate(meta.legalTextKey, undefined, lang); switch (c.type) { case 'USE_CASE': { - const labels = c.useCases.map((id) => { - const key = `useCase.${id.replace(/^UC\./, '')}`; - return transloco.translate(key, undefined, lang); - }); + const labels = c.useCases.map((id) => useCaseLabel(id, transloco, lang)); return transloco.translate( 'legalDescription.clause.useCase', { list: joinList(labels) }, @@ -99,7 +121,7 @@ function buildClause(c: Constraint, transloco: TranslocoService, lang?: string): case 'FRAMEWORK_AGREEMENT': return transloco.translate( 'legalDescription.clause.frameworkAgreement', - { agreement: c.agreement }, + { agreement: c.agreement === FRAMEWORK_AGREEMENT_VALUE ? c.agreement : UNKNOWN_VALUE }, lang, ); case 'MEMBERSHIP': @@ -108,13 +130,24 @@ function buildClause(c: Constraint, transloco: TranslocoService, lang?: string): } } +/** + * Übersetzt eine Use-Case-ID über die Registry. Der i18n-Key wird bewusst NICHT aus der ID + * zusammengesetzt, sondern der Registry entnommen — sonst könnte eine ID aus dem Backend + * einen beliebigen Key erzeugen (`useCase.`), dessen Auflösung bei fehlendem Key + * den Key selbst zurückliefert und ihn so in den Rechtstext schreibt. + */ +function useCaseLabel(id: string, transloco: TranslocoService, lang?: string): string { + const option = USE_CASE_OPTIONS.find((o) => o.id === id); + return option ? transloco.translate(option.labelKey, undefined, lang) : UNKNOWN_VALUE; +} + function joinList(items: string[]): string { if (items.length <= 1) return items.join(''); return items.slice(0, -1).join(', ') + ' & ' + items[items.length - 1]; } function formatDate(iso: string): string { - if (!iso) return '—'; + if (!iso) return UNKNOWN_VALUE; // Reine Datumsangaben (YYYY-MM-DD, das Format der DATE_RANGE-Eingabe) direkt formatieren, // ohne sie durch `new Date()` in UTC-Mitternacht zu wandeln. Sonst kippt der lokale Tag // in Zeitzonen westlich von UTC um einen Tag — beim rechtlich maßgeblichen Text unzulässig. @@ -124,7 +157,9 @@ function formatDate(iso: string): string { return `${day}.${month}.${year}`; } const d = new Date(iso); - if (isNaN(d.getTime())) return iso; + // Kein Rohwert-Fallback: ein unparsebarer Wert würde sonst ungeprüft als + // Transloco-Parameter in den Rechtstext gelangen (siehe Hinweis an `buildClause`). + if (isNaN(d.getTime())) return UNKNOWN_VALUE; const day = String(d.getDate()).padStart(2, '0'); const month = String(d.getMonth() + 1).padStart(2, '0'); const year = d.getFullYear(); From bde63dfc5dc7bd02317354a1eb000b83e3a332aa Mon Sep 17 00:00:00 2001 From: Mario Haefs Date: Thu, 6 Aug 2026 12:20:13 +0200 Subject: [PATCH 04/12] fix(services): url-encode the policy id in API paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `paramMap.get('id')` is input, not a trusted identifier. Angular's DefaultUrlSerializer decodes %2F and %2E after splitting the path into segments, so a route parameter can contain slashes and dot segments. Verified against the installed @angular/router: /policies/..%2F..%2Factuator%2Fenv -> segments ["policies", "../../actuator/env"] /policies/%2E%2E%2F%2E%2E%2Fadmin/edit -> segments ["policies", "../../admin", "edit"] (matches :id/edit) The service interpolated that straight into a template string, and the browser normalises the result before sending: new URL('/api/v1/policies/../../actuator/env', origin).pathname -> '/api/actuator/env' So a crafted link made the app issue a same-origin request to an arbitrary /api endpoint with the user's credentials — and on the edit route a single click on "Save" turned that into a PUT. Being same-origin, neither CORS nor SameSite applies. All three id-bearing paths now go through a shared resourceUrl() helper that applies encodeURIComponent. Tests assert the invariant (the resolved path stays below /v1/policies/) and fail without the fix. Co-Authored-By: Claude Opus 5 (1M context) --- .../services/policies/policy.service.spec.ts | 90 +++++++++++++++++++ .../app/services/policies/policy.service.ts | 17 +++- 2 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 frontend/src/app/services/policies/policy.service.spec.ts diff --git a/frontend/src/app/services/policies/policy.service.spec.ts b/frontend/src/app/services/policies/policy.service.spec.ts new file mode 100644 index 0000000..8703d47 --- /dev/null +++ b/frontend/src/app/services/policies/policy.service.spec.ts @@ -0,0 +1,90 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { HttpClient } from '@angular/common/http'; +import { TestBed } from '@angular/core/testing'; +import { Observable, of } from 'rxjs'; + +import { PolicyService } from './policy.service'; + +/** + * Der Route-Parameter `id` kommt aus `paramMap.get('id')` und ist damit reine Eingabe: + * Angulars `DefaultUrlSerializer` dekodiert `%2F` zu `/` und `%2E` zu `.`, nachdem er den + * Pfad in Segmente zerlegt hat. `/policies/..%2F..%2Factuator%2Fenv` liefert also + * `id = '../../actuator/env'`. Ohne Kodierung baut der Service daraus + * `/api/v1/policies/../../actuator/env`, was der Browser vor dem Absenden zu + * `/api/actuator/env` normalisiert — der Request verlässt die Policies-Collection. + * + * Invariante: egal welche ID hereinkommt, der aufgelöste Pfad muss unterhalb von + * `/v1/policies/` bleiben. + */ +describe('PolicyService — Pfad-Konstruktion', () => { + const TRAVERSAL_ID = '../../actuator/env'; + + // Signaturen als Generic statt als Parameterliste: so ist `mock.calls` typisiert, + // ohne ungenutzte Parameter zu deklarieren (ESLint `no-unused-vars`). + function makeHttp() { + return { + get: vi.fn<(url: string) => Observable>(() => of({})), + post: vi.fn<(url: string, body: unknown) => Observable>(() => of({})), + put: vi.fn<(url: string, body: unknown) => Observable>(() => of({})), + delete: vi.fn<(url: string) => Observable>(() => of(undefined)), + }; + } + + let http: ReturnType; + let service: PolicyService; + + beforeEach(() => { + http = makeHttp(); + TestBed.configureTestingModule({ providers: [{ provide: HttpClient, useValue: http }] }); + service = TestBed.inject(PolicyService); + }); + + /** Bildet die Pfad-Normalisierung nach, die der Browser vor dem Absenden vornimmt. */ + function resolvedPath(url: string): string { + return new URL(url, 'https://policy-hub.example').pathname; + } + + function collectionPath(): string { + return resolvedPath(service['baseUrl']) + '/'; + } + + it('getPolicyById kodiert die ID und bleibt in der Policies-Collection', () => { + service.getPolicyById(TRAVERSAL_ID).subscribe(); + + const url = http.get.mock.calls[0][0]; + expect(url).toContain(encodeURIComponent(TRAVERSAL_ID)); + expect(resolvedPath(url).startsWith(collectionPath())).toBe(true); + }); + + it('updatePolicy kodiert die ID und bleibt in der Policies-Collection', () => { + service + .updatePolicy(TRAVERSAL_ID, { + policyId: 'x', + category: 'ACCESS', + constraints: [], + legalText: '', + }) + .subscribe(); + + const url = http.put.mock.calls[0][0]; + expect(url).toContain(encodeURIComponent(TRAVERSAL_ID)); + expect(resolvedPath(url).startsWith(collectionPath())).toBe(true); + }); + + it('deletePolicy kodiert die ID und bleibt in der Policies-Collection', () => { + service.deletePolicy(TRAVERSAL_ID).subscribe(); + + const url = http.delete.mock.calls[0][0]; + expect(url).toContain(encodeURIComponent(TRAVERSAL_ID)); + expect(resolvedPath(url).startsWith(collectionPath())).toBe(true); + }); + + it('lässt eine unauffällige UUID unverändert', () => { + const id = '00000000-0000-0000-0000-000000000001'; + service.getPolicyById(id).subscribe(); + + const url = http.get.mock.calls[0][0]; + expect(url.endsWith(`/v1/policies/${id}`)).toBe(true); + }); +}); diff --git a/frontend/src/app/services/policies/policy.service.ts b/frontend/src/app/services/policies/policy.service.ts index 195a540..aa6eb64 100644 --- a/frontend/src/app/services/policies/policy.service.ts +++ b/frontend/src/app/services/policies/policy.service.ts @@ -9,12 +9,23 @@ export class PolicyService { private readonly http = inject(HttpClient); private readonly baseUrl = `${environment.backendUrl}/v1/policies`; + /** + * IDs stammen aus `paramMap.get('id')` und sind damit Eingabe: Angulars UrlSerializer + * dekodiert `%2F`/`%2E`, sodass eine ID `/` und `..` enthalten kann. Ohne Kodierung würde + * der Browser den fertigen Pfad normalisieren (`…/policies/../../actuator` → `/api/actuator`) + * und der Request die Policies-Collection verlassen — same-origin und mit den Credentials + * des Nutzers. Deshalb geht jede ID durch `encodeURIComponent`. + */ + private resourceUrl(id: string): string { + return `${this.baseUrl}/${encodeURIComponent(id)}`; + } + getAllPolicies(): Observable { return this.http.get(this.baseUrl); } getPolicyById(id: string): Observable { - return this.http.get(`${this.baseUrl}/${id}`); + return this.http.get(this.resourceUrl(id)); } createPolicy(request: CreatePolicyRequest): Observable { @@ -22,10 +33,10 @@ export class PolicyService { } updatePolicy(id: string, request: UpdatePolicyRequest): Observable { - return this.http.put(`${this.baseUrl}/${id}`, request); + return this.http.put(this.resourceUrl(id), request); } deletePolicy(id: string): Observable { - return this.http.delete(`${this.baseUrl}/${id}`); + return this.http.delete(this.resourceUrl(id)); } } From 1690f5dd9844f7caf3c4b943d8b44c109de34bc3 Mon Sep 17 00:00:00 2001 From: Mario Haefs Date: Thu, 6 Aug 2026 12:51:40 +0200 Subject: [PATCH 05/12] fix(policy-builder): handle unknown constraint types from the API CONSTRAINT_METADATA is typed as Record, which only holds at compile time. Constraints are also read from GET /v1/policies/:id, so the type is an unverified claim about runtime data. For an unrecognised type the index access returns undefined and the following property read throws mid-render: TypeError: Cannot read properties of undefined (reading 'labelKey') TypeError: Cannot read properties of undefined (reading 'allowedIn') A single stored policy with {"type":"FOO"} therefore left the detail page broken for every user until the record was removed. Adds isKnownConstraintType() (hasOwnProperty, so inherited Object members like `constructor` are not mistaken for registry entries) and keepKnownConstraints(). Unknown types are now dropped at the two load boundaries with a warning to the user, buildLegalClauses() skips them instead of aborting the whole legal text, and validatePolicyDraft() reports them as a validation error. New i18n keys in de.json and en.json: validation.constraintUnknownType, policyDetail.notifications. unknownConstraints, policyEditor.notifications.unknownConstraints. Co-Authored-By: Claude Opus 5 (1M context) --- .../policy-detail-page.component.ts | 18 ++++++- .../helpers/legal-description.helper.spec.ts | 24 ++++++++++ .../helpers/legal-description.helper.ts | 14 ++++-- .../metadata/constraint-metadata.spec.ts | 47 ++++++++++++++++++- .../metadata/constraint-metadata.ts | 24 ++++++++-- .../validators/constraint-validators.spec.ts | 26 ++++++++++ .../validators/constraint-validators.ts | 16 ++++++- .../policy-editor-page.component.ts | 12 ++++- frontend/src/assets/i18n/de.json | 9 ++-- frontend/src/assets/i18n/en.json | 9 ++-- 10 files changed, 182 insertions(+), 17 deletions(-) diff --git a/frontend/src/app/pages/policies/policy-detail-page/policy-detail-page.component.ts b/frontend/src/app/pages/policies/policy-detail-page/policy-detail-page.component.ts index a9a4e4f..db1d6db 100644 --- a/frontend/src/app/pages/policies/policy-detail-page/policy-detail-page.component.ts +++ b/frontend/src/app/pages/policies/policy-detail-page/policy-detail-page.component.ts @@ -22,6 +22,7 @@ import { ConfirmDeleteDialogComponent } from '@ui/confirm-delete-dialog/confirm- import { ConstraintCardComponent } from '@features/policies/builder/components/constraint-card/constraint-card.component'; import { policyToOdrl } from '@services/policies/policy-mapper/policy-odrl.mapper'; import { buildLegalClauses } from '@features/policies/builder/helpers/legal-description.helper'; +import { keepKnownConstraints } from '@features/policies/builder/metadata/constraint-metadata'; @Component({ selector: 'app-policy-detail-page', @@ -78,7 +79,7 @@ export class PolicyDetailPageComponent implements OnInit { } this.policyService.getPolicyById(id).subscribe({ next: (data) => { - this.policy.set(data); + this.policy.set(this.withKnownConstraintsOnly(data)); this.loading.set(false); }, error: () => { @@ -89,6 +90,21 @@ export class PolicyDetailPageComponent implements OnInit { }); } + /** + * Verwirft Constraints, deren Typ die Metadaten-Registry nicht kennt, und weist den + * Nutzer darauf hin. Ohne diesen Filter würde `app-constraint-card` beim Rendern auf + * `undefined` zugreifen und die gesamte Detailseite mit einem TypeError abbrechen. + */ + private withKnownConstraintsOnly(policy: Policy): Policy { + const constraints = keepKnownConstraints(policy.constraints); + if (constraints.length !== policy.constraints.length) { + this.notification.warning( + this.transloco.translate('policyDetail.notifications.unknownConstraints'), + ); + } + return { ...policy, constraints }; + } + deletePolicy(): void { const p = this.policy(); if (!p) return; diff --git a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.spec.ts b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.spec.ts index da5d91f..f0c2976 100644 --- a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.spec.ts +++ b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.spec.ts @@ -299,3 +299,27 @@ describe('buildLegalDescription', () => { }); }); }); + +describe('buildLegalClauses — unbekannter Constraint-Typ aus der API', () => { + const unknownConstraint = { type: 'FOO' } as unknown as Constraint; + + it('übergeht den unbekannten Typ, statt beim Metadaten-Zugriff zu werfen', () => { + const { clauses } = buildLegalClauses( + draft('CONTRACT', [unknownConstraint, { type: 'MEMBERSHIP', value: 'active' }]), + makeTransloco(), + ); + + expect(clauses).toHaveLength(1); + expect(clauses[0].title).toBe('constraint.MEMBERSHIP.label'); + }); + + it('fällt auf den "unrestricted"-Text zurück, wenn nur unbekannte Typen übrig bleiben', () => { + const { intro, clauses } = buildLegalClauses( + draft('CONTRACT', [unknownConstraint]), + makeTransloco(), + ); + + expect(intro).toBe('legalDescription.unrestricted'); + expect(clauses).toEqual([]); + }); +}); diff --git a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.ts b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.ts index c2be1c7..3f21ca7 100644 --- a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.ts +++ b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/helpers/legal-description.helper.ts @@ -1,7 +1,10 @@ import { TranslocoService } from '@jsverse/transloco'; import { Constraint } from '@shared/types/constraint.model'; import { Policy } from '@shared/types/policy.model'; -import { CONSTRAINT_METADATA } from '@features/policies/builder/metadata/constraint-metadata'; +import { + CONSTRAINT_METADATA, + keepKnownConstraints, +} from '@features/policies/builder/metadata/constraint-metadata'; import { FRAMEWORK_AGREEMENT_VALUE, USE_CASE_OPTIONS, @@ -40,7 +43,12 @@ export function buildLegalClauses( transloco: TranslocoService, lang?: string, ): LegalClauses { - if (!policy.constraints.length) { + // Constraints mit unbekanntem Typ übergehen: sie haben keinen Eintrag in der + // Metadaten-Registry, und der Zugriff auf `labelKey`/`legalTextKey` würde werfen. + // Für den Rechtstext ist Weglassen richtiger als ein Abbruch der ganzen Darstellung. + const constraints = keepKnownConstraints(policy.constraints); + + if (!constraints.length) { return { intro: transloco.translate('legalDescription.unrestricted', undefined, lang), clauses: [], @@ -55,7 +63,7 @@ export function buildLegalClauses( lang, ); - const clauses = policy.constraints.map((c) => ({ + const clauses = constraints.map((c) => ({ title: transloco.translate(CONSTRAINT_METADATA[c.type].labelKey, undefined, lang), text: buildClause(c, transloco, lang), })); diff --git a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/metadata/constraint-metadata.spec.ts b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/metadata/constraint-metadata.spec.ts index a89c2ca..a3ca840 100644 --- a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/metadata/constraint-metadata.spec.ts +++ b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/metadata/constraint-metadata.spec.ts @@ -1,6 +1,14 @@ import { describe, expect, it } from 'vitest'; -import { buildDefaultConstraint, getAllowedConstraintTypes } from './constraint-metadata'; +import { Constraint } from '@shared/types/constraint.model'; + +import { + ALL_CONSTRAINT_TYPES, + buildDefaultConstraint, + getAllowedConstraintTypes, + isKnownConstraintType, + keepKnownConstraints, +} from './constraint-metadata'; describe('getAllowedConstraintTypes', () => { it('erlaubt alle vier Typen (inkl. DATE_RANGE) für ACCESS', () => { @@ -46,3 +54,40 @@ describe('buildDefaultConstraint', () => { }); }); }); + +/** + * `CONSTRAINT_METADATA` ist als `Record` typisiert — das gilt aber nur + * zur Compile-Zeit. Constraints kommen auch aus `GET /v1/policies/:id` und sind damit + * ungeprüfte Eingabe; ein unbekannter Typ liefert beim Index-Zugriff `undefined`. + */ +describe('isKnownConstraintType / keepKnownConstraints', () => { + it('erkennt alle registrierten Typen', () => { + for (const type of ALL_CONSTRAINT_TYPES) { + expect(isKnownConstraintType(type)).toBe(true); + } + }); + + it('weist einen unbekannten Typ ab', () => { + expect(isKnownConstraintType('FOO')).toBe(false); + }); + + it('lässt sich nicht von geerbten Object-Properties täuschen', () => { + // Ohne hasOwnProperty würde `'constructor' in CONSTRAINT_METADATA` true liefern. + expect(isKnownConstraintType('constructor')).toBe(false); + expect(isKnownConstraintType('toString')).toBe(false); + expect(isKnownConstraintType('__proto__')).toBe(false); + }); + + it('filtert unbekannte Constraints heraus und erhält die bekannten', () => { + const constraints = [ + { type: 'MEMBERSHIP', value: 'active' }, + { type: 'FOO' }, + { type: 'DATE_RANGE', startDate: '2027-01-01', endDate: '2027-12-31' }, + ] as unknown as Constraint[]; + + expect(keepKnownConstraints(constraints)).toEqual([ + { type: 'MEMBERSHIP', value: 'active' }, + { type: 'DATE_RANGE', startDate: '2027-01-01', endDate: '2027-12-31' }, + ]); + }); +}); diff --git a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/metadata/constraint-metadata.ts b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/metadata/constraint-metadata.ts index a8be3ad..f79e2c8 100644 --- a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/metadata/constraint-metadata.ts +++ b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/metadata/constraint-metadata.ts @@ -1,4 +1,4 @@ -import { ConstraintType, Operator } from '@shared/types/constraint.model'; +import { Constraint, ConstraintType, Operator } from '@shared/types/constraint.model'; import { PolicyCategory } from '@shared/types/policy.model'; export interface ConstraintMetadata { @@ -61,9 +61,25 @@ export function getAllowedConstraintTypes(category: PolicyCategory): ConstraintT return ALL_CONSTRAINT_TYPES.filter((t) => CONSTRAINT_METADATA[t].allowedIn.includes(category)); } -export function buildDefaultConstraint( - type: ConstraintType, -): import('@shared/types/constraint.model').Constraint { +/** + * Laufzeit-Prüfung, ob die Registry einen Constraint-Typ kennt. + * + * `Record` verspricht nur zur Compile-Zeit, dass ein + * Index-Zugriff Metadaten liefert. Constraints aus `GET /v1/policies/:id` sind ungeprüfte + * Eingabe: bei einem unbekannten Typ ist `CONSTRAINT_METADATA[type]` zur Laufzeit `undefined`, + * und der folgende Property-Zugriff (`.icon`, `.labelKey`, `.allowedIn`) wirft einen TypeError + * mitten im Rendering — die Seite bleibt für jeden Nutzer defekt. + */ +export function isKnownConstraintType(type: string): type is ConstraintType { + return Object.prototype.hasOwnProperty.call(CONSTRAINT_METADATA, type); +} + +/** Entfernt alle Constraints, deren Typ die Registry nicht kennt. */ +export function keepKnownConstraints(constraints: readonly Constraint[]): Constraint[] { + return constraints.filter((c) => isKnownConstraintType(c.type)); +} + +export function buildDefaultConstraint(type: ConstraintType): Constraint { switch (type) { case 'MEMBERSHIP': return { type: 'MEMBERSHIP', value: 'active' }; diff --git a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.spec.ts b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.spec.ts index d34e0a0..12ec341 100644 --- a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.spec.ts +++ b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.spec.ts @@ -198,3 +198,29 @@ describe('validatePolicyDraft', () => { expect(keys).toContain('validation.useCaseRequired'); }); }); + +describe('validatePolicyDraft — unbekannter Constraint-Typ aus der API', () => { + const unknownConstraint = { type: 'FOO' } as unknown as Constraint; + + it('meldet einen Validierungsfehler statt zu werfen', () => { + const errors = validatePolicyDraft({ + policyId: 'gueltige-id', + category: 'ACCESS', + constraints: [unknownConstraint], + } as Partial); + + expect(errors.map((e) => e.messageKey)).toContain('validation.constraintUnknownType'); + }); + + it('prüft die übrigen Constraints trotzdem weiter', () => { + const errors = validatePolicyDraft({ + policyId: 'gueltige-id', + category: 'ACCESS', + constraints: [unknownConstraint, { type: 'USE_CASE', useCases: [] }], + } as Partial); + + const keys = errors.map((e) => e.messageKey); + expect(keys).toContain('validation.constraintUnknownType'); + expect(keys).toContain('validation.useCaseRequired'); + }); +}); diff --git a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.ts b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.ts index 18a7bd7..9447cb3 100644 --- a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.ts +++ b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.ts @@ -1,6 +1,9 @@ import { Constraint } from '@shared/types/constraint.model'; import { Policy } from '@shared/types/policy.model'; -import { CONSTRAINT_METADATA } from '@features/policies/builder/metadata/constraint-metadata'; +import { + CONSTRAINT_METADATA, + isKnownConstraintType, +} from '@features/policies/builder/metadata/constraint-metadata'; export interface ValidationError { field: string; @@ -22,6 +25,17 @@ export function validatePolicyDraft(draft: Partial): ValidationError[] { } for (const [index, c] of (draft.constraints ?? []).entries()) { + // Typ zuerst gegen die Registry prüfen: bei einem unbekannten Typ (möglich, weil + // Constraints auch aus der API stammen) liefert der Index-Zugriff `undefined` und + // `.allowedIn` würde werfen. Ein unbekannter Typ ist ein Validierungsfehler, kein Absturz. + if (!isKnownConstraintType(c.type)) { + errors.push({ + field: `constraint[${index}]`, + messageKey: 'validation.constraintUnknownType', + }); + continue; + } + // Check category compatibility if (draft.category && !CONSTRAINT_METADATA[c.type].allowedIn.includes(draft.category)) { errors.push({ diff --git a/frontend/src/app/pages/policies/policy-editor-page/policy-editor-page.component.ts b/frontend/src/app/pages/policies/policy-editor-page/policy-editor-page.component.ts index ee4a447..2bfa8a1 100644 --- a/frontend/src/app/pages/policies/policy-editor-page/policy-editor-page.component.ts +++ b/frontend/src/app/pages/policies/policy-editor-page/policy-editor-page.component.ts @@ -4,6 +4,7 @@ import { TranslocoDirective, TranslocoService } from '@jsverse/transloco'; import { PolicyService } from '@services/policies/policy.service'; import { NotificationService } from '@services/notification/notification.service'; import { Policy } from '@shared/types/policy.model'; +import { keepKnownConstraints } from '@features/policies/builder/metadata/constraint-metadata'; import { PolicyBuilderComponent, PolicyDraft, @@ -37,7 +38,16 @@ export class PolicyEditorPageComponent implements OnInit { this.loading.set(true); this.policyService.getPolicyById(id).subscribe({ next: (p) => { - this.initialPolicy.set(p); + // Unbekannte Constraint-Typen entfernen, bevor sie in den Builder gelangen: + // sie haben keinen Registry-Eintrag und würden beim Rendern der Editor-Card + // bzw. in der Validierung auf `undefined` zugreifen. + const constraints = keepKnownConstraints(p.constraints); + if (constraints.length !== p.constraints.length) { + this.notification.warning( + this.transloco.translate('policyEditor.notifications.unknownConstraints'), + ); + } + this.initialPolicy.set({ ...p, constraints }); this.loading.set(false); }, error: () => { diff --git a/frontend/src/assets/i18n/de.json b/frontend/src/assets/i18n/de.json index e6f21bf..beb6f03 100644 --- a/frontend/src/assets/i18n/de.json +++ b/frontend/src/assets/i18n/de.json @@ -142,7 +142,8 @@ "notifications": { "loadError": "Policy konnte nicht geladen werden.", "deleteSuccess": "Policy wurde gelöscht.", - "deleteError": "Policy konnte nicht gelöscht werden." + "deleteError": "Policy konnte nicht gelöscht werden.", + "unknownConstraints": "Die Policy enthält unbekannte Bedingungstypen. Diese werden nicht angezeigt." } }, "policyEditor": { @@ -151,7 +152,8 @@ "createSuccess": "Policy wurde erstellt.", "createError": "Policy konnte nicht erstellt werden.", "updateSuccess": "Policy wurde aktualisiert.", - "updateError": "Policy konnte nicht aktualisiert werden." + "updateError": "Policy konnte nicht aktualisiert werden.", + "unknownConstraints": "Die Policy enthält unbekannte Bedingungstypen. Diese wurden aus dem Editor entfernt." } }, "policyBuilder": { @@ -271,7 +273,8 @@ "dateRangeEndRequired": "Enddatum ist erforderlich", "dateRangeEndInvalid": "Enddatum ist ungültig", "dateRangeEndInPast": "Enddatum muss in der Zukunft liegen", - "dateRangeStartAfterEnd": "Das Startdatum muss vor dem Enddatum liegen" + "dateRangeStartAfterEnd": "Das Startdatum muss vor dem Enddatum liegen", + "constraintUnknownType": "Unbekannter Bedingungstyp — diese Bedingung wird ignoriert." }, "deleteDialog": { "title": "Policy löschen", diff --git a/frontend/src/assets/i18n/en.json b/frontend/src/assets/i18n/en.json index 05067ab..6a0697c 100644 --- a/frontend/src/assets/i18n/en.json +++ b/frontend/src/assets/i18n/en.json @@ -142,7 +142,8 @@ "notifications": { "loadError": "Policy could not be loaded.", "deleteSuccess": "Policy deleted.", - "deleteError": "Policy could not be deleted." + "deleteError": "Policy could not be deleted.", + "unknownConstraints": "This policy contains unknown condition types. They are not displayed." } }, "policyEditor": { @@ -151,7 +152,8 @@ "createSuccess": "Policy created.", "createError": "Policy could not be created.", "updateSuccess": "Policy updated.", - "updateError": "Policy could not be updated." + "updateError": "Policy could not be updated.", + "unknownConstraints": "This policy contains unknown condition types. They were removed from the editor." } }, "policyBuilder": { @@ -271,7 +273,8 @@ "dateRangeEndRequired": "End date is required", "dateRangeEndInvalid": "End date is invalid", "dateRangeEndInPast": "End date must be in the future", - "dateRangeStartAfterEnd": "The start date must be before the end date" + "dateRangeStartAfterEnd": "The start date must be before the end date", + "constraintUnknownType": "Unknown condition type — this condition is ignored." }, "deleteDialog": { "title": "Delete policy", From 0ac8a1d29ae841707a1d6e1eb5b82370ffc6b44c Mon Sep 17 00:00:00 2001 From: Mario Haefs Date: Thu, 6 Aug 2026 13:20:33 +0200 Subject: [PATCH 06/12] fix(policy-builder): restrict policyId to a safe character set validatePolicyDraft() only checked that the id was non-empty and at most 200 characters. The value is taken over verbatim by policyToOdrl() as the JSON-LD @id of the emitted PolicyDefinition. @id is an identifier, not a label. An unconstrained string can therefore produce an absolute IRI (https://w3id.org/.../another-policy) or a blank node reference (_:b0) and point the document at a different resource than the one being edited. Adds a slug pattern (letter or digit first, then also dot, underscore, hyphen) which covers the documented format "policy.use-case-quality-assurance" and every id in the mock dataset and E2E specs. Reported as validation.policyIdInvalidChars, added to de.json and en.json. The check runs after the required/too-long checks so an empty id still reports policyIdRequired rather than a confusing charset error. Co-Authored-By: Claude Opus 5 (1M context) --- .../validators/constraint-validators.spec.ts | 49 +++++++++++++++++++ .../validators/constraint-validators.ts | 13 +++++ frontend/src/assets/i18n/de.json | 3 +- frontend/src/assets/i18n/en.json | 3 +- 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.spec.ts b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.spec.ts index 12ec341..d048e4c 100644 --- a/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.spec.ts +++ b/frontend/src/app/pages/policies/policy-editor-page/policy-builder/validators/constraint-validators.spec.ts @@ -224,3 +224,52 @@ describe('validatePolicyDraft — unbekannter Constraint-Typ aus der API', () => expect(keys).toContain('validation.useCaseRequired'); }); }); + +/** + * Die policyId landet über `policyToOdrl()` als JSON-LD-`@id` im ODRL-Dokument. `@id` ist + * dort ein Identifier, keine Beschriftung: eine freie Zeichenkette könnte eine absolute IRI + * oder eine Blank-Node-Referenz erzeugen und damit auf eine fremde PolicyDefinition zeigen. + */ +describe('validatePolicyDraft — Zeichensatz der policyId', () => { + function withId(policyId: string): Partial { + return { policyId, category: 'ACCESS', constraints: [] }; + } + + const accepted = [ + 'zugriff-konsortium-mitglieder', + 'policy.use-case-quality-assurance', + 'bim_koordination_2027', + 'e2e-neue-policy', + 'A1', + ]; + + for (const id of accepted) { + it(`akzeptiert die fachliche ID "${id}"`, () => { + expect(validatePolicyDraft(withId(id))).toEqual([]); + }); + } + + const rejected: [string, string][] = [ + ['absolute IRI', 'https://w3id.org/catenax/policy/fremde-policy'], + ['Blank-Node-Referenz', '_:b0'], + ['Leerzeichen', 'policy mit leerzeichen'], + ['Pfadtrenner', '../andere-policy'], + ['führender Punkt', '.versteckt'], + ['spitze Klammern', 'policy