EDM-2954: Allow configuring app privileges #460
Conversation
WalkthroughAdds optional runAs fields to application types and device models, exposes run-as controls in the device edit UI via a new ApplicationIntegritySettings component, propagates runAs through device-spec utilities and validation, adds translations, and displays a "Run as user" column in the applications table. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Edit Device UI
participant Formik as Formik State
participant Utils as deviceSpecUtils
participant API as API Model
User->>UI: Toggle "System integrity" switch
UI->>Formik: setFieldValue("applications[i].runAs", runAsValue)
Formik->>UI: render updated fields (rootless identity shown/hidden)
User->>UI: Submit form
UI->>Utils: getApplicationValues(formState)
Utils->>Utils: create*App helpers attach runAs to payload
Utils->>API: return serialized application payload including runAs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
celdrake
force-pushed
the
EDM-2954-app-privilege-configuration
branch
2 times, most recently
from
dc8e398 to
b484007
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@libs/types/models/DeviceApplicationStatus.ts`:
- Around line 27-30: Fix the JSDoc typo for the runAs property in
DeviceApplicationStatus: change "runing" to "running" in the comment above the
runAs?: string; declaration so the description reads "...this application is
running under."
In `@libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx`:
- Line 48: The UI is showing RUN_AS_DEFAULT_USER ('flightctl') when
DeviceApplicationStatus.runAs is undefined, which conflicts with the API doc
stating the agent's user (generally root) is used; update ApplicationsTable.tsx
(the Td rendering using app.runAs || RUN_AS_DEFAULT_USER) and the application
form components (Container, Image, Inline) and helper text to reflect the API
fallback (display the agent user, e.g., "agent (root)") when runAs is undefined
OR, if the runtime default truly is 'flightctl', change the RUN_AS_DEFAULT_USER
constant and related helper text so the UI and API documentation are
consistent—make the chosen single source of truth the value shown whenever
app.runAs is falsy.
In
`@libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts`:
- Around line 320-332: In toAPIApplication, the call to toInlineFiles(app.files)
is wrong because toInlineFiles converts API->form; replace it with a form->API
converter (e.g., toAPIInlineFiles) that transforms each file's base64 boolean
into the API's contentEncoding: EncodingType and preserves name/content; if such
a helper doesn't exist, implement toAPIInlineFiles(files) and use it when
building inlineData.inline, and ensure Quadlet-related runAs handling remains
unchanged.
🧹 Nitpick comments (3)
libs/ui-components/src/components/form/validations.ts (1)
707-707: Consider addingrunAsvalidation to Quadlet app schemas for consistency.The
runAsfield has been added to the validation schema forSingleContainerAppForm, but looking atdeviceSpec.ts, therunAs?: stringproperty is also present onQuadletImageAppForm(line 76) andQuadletInlineAppForm(line 84).For consistency and to prevent potential validation issues, consider adding
runAs: Yup.string()to the Quadlet validation schemas as well (around lines 743-756 and 761-771).♻️ Suggested additions
For the image applications schema (around line 756):
volumes: composeQuadletVolumesSchema(t), variables: appVariablesSchema(t), + runAs: Yup.string(), });For the inline quadlet applications schema (around line 771):
volumes: composeQuadletVolumesSchema(t), variables: appVariablesSchema(t), + runAs: Yup.string(), });libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (2)
638-647: Consider renamingtoInlineFilesor adding a complementary function.The function
toInlineFilesconverts from API format (contentEncoding) to form format (base64), but the name doesn't clearly indicate the conversion direction. This ambiguity contributed to its incorrect usage at line 324.Consider either:
- Renaming to
apiFilesToFormFilesfor clarity- Adding a complementary
formFilesToApiFilesfunction for the reverse conversion
297-302: Minor inconsistency:runAsalways set for Single Container but conditionally for Quadlet.For Single Container apps (line 300),
runAsis always included in the API payload with a default ofRUN_AS_DEFAULT_USER. For Quadlet apps (lines 314-316, 328-330),runAsis only included if explicitly defined.This works correctly because the
create*Apphelpers always initializerunAs, but the conditional pattern for Quadlet could be simplified to match the Single Container pattern for consistency.Optional: Simplify to match Single Container pattern
if (isQuadletImageAppForm(app) || isComposeImageAppForm(app)) { const data: ApplicationProviderSpec = { image: app.image, appType: app.appType, envVars, volumes, }; if (app.name) { data.name = app.name; } - if (isQuadletImageAppForm(app) && app.runAs) { - data.runAs = app.runAs; + if (isQuadletImageAppForm(app)) { + data.runAs = app.runAs || RUN_AS_DEFAULT_USER; } return data; }
libs/ui-components/src/components/DetailsPage/Tables/ApplicationsTable.tsx
Show resolved
Hide resolved
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts
Show resolved
Hide resolved
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationContainerForm.tsx
Outdated
Show resolved
Hide resolved
celdrake
force-pushed
the
EDM-2954-app-privilege-configuration
branch
from
bc86be2 to
c457348
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@libs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationInlineForm.tsx`:
- Line 2: The import statement in ApplicationInlineForm.tsx includes an unused
symbol `useField`; remove `useField` from the import list so only `FieldArray`
is imported (i.e., change the import that currently reads "import { FieldArray,
useField } from 'formik';" to import only `FieldArray`) to satisfy the static
analysis and avoid unused-import warnings.
🧹 Nitpick comments (1)
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (1)
798-811: Consider defining a proper type for the intersection.The cast at line 806 (
as ApplicationProviderSpec & InlineApplicationProviderSpec & { runAs?: string }) suggests the base types don't includerunAsfor inline applications. While functional, this inline type extension is a maintenance risk.Consider whether the
@flightctl/typesdefinitions should be updated to includerunAson the appropriate specs, or define a local type alias for clarity.
libs/ui-components/src/components/Device/EditDeviceWizard/steps/ApplicationInlineForm.tsx
Outdated
Show resolved
Hide resolved
celdrake
force-pushed
the
EDM-2954-app-privilege-configuration
branch
2 times, most recently
from
ce1c0e7 to
ee46cbf
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (1)
300-331: InconsistentrunAsserialization creates privilege escalation risk for legacy appsBackend defaults omitted
runAsto root (agent user), but the UI defaults to 'flightctl'. This creates a mismatch when editing existing apps:SingleContainerApp (line 300) always serializes with the fallback:
data.runAs = app.runAs || RUN_AS_DEFAULT_USER, so any edit to an app with undefinedrunAs(currently root on backend) will change it to 'flightctl'.QuadletImageApp and QuadletInlineApp correctly avoid this by only serializing runAs when explicitly defined (lines 314 and 321), but form initialization (lines 709, 720, 728) still defaults all app types to 'flightctl' when loading undefined values.
Change detection (lines 354, 463, 522) also uses the fallback pattern, masking actual privilege changes.
Preserve the backend's undefined
runAsfor existing apps unless the user explicitly changes the setting. Only default 'flightctl' in the form for new apps.
celdrake
force-pushed
the
EDM-2954-app-privilege-configuration
branch
from
ee46cbf to
18d97a5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts`:
- Around line 798-811: The switch on app.appType (inside the map callback that
returns app specs) lacks a default and can return undefined; update that switch
to include a default branch that throws a descriptive Error for unexpected
AppType values (e.g., referencing app.appType and maybe app.id) so the callback
always returns or fails fast. Ensure you modify the same switch that calls
createContainerApp, createHelmApp, createQuadletImageApp,
createQuadletInlineApp, createComposeImageApp and createComposeInlineApp and
keep existing isImageAppProvider checks intact.
🧹 Nitpick comments (1)
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts (1)
771-789: Add a default branch to guard unknownAppTypevalues.Future enum additions could leave
appuninitialized; a default throw keeps failures explicit.Suggested update
switch (appType) { case AppType.AppTypeContainer: app = createContainerApp(undefined); break; case AppType.AppTypeHelm: app = createHelmApp(undefined); break; case AppType.AppTypeQuadlet: app = specType === AppSpecType.OCI_IMAGE ? createQuadletImageApp(undefined) : createQuadletInlineApp(undefined); break; case AppType.AppTypeCompose: app = specType === AppSpecType.OCI_IMAGE ? createComposeImageApp(undefined) : createComposeInlineApp(undefined); break; + default: + throw new Error(`Unsupported appType: ${appType}`); }
libs/ui-components/src/components/Device/EditDeviceWizard/deviceSpecUtils.ts
Show resolved
Hide resolved
2 participants
Adds workload privilege settings to Single Container and Quadlets applications.
Summary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.