Rework the function editor demo - #17
AI Code Review Results
AI Pull Request Overview
Disclaimer: This review was generated by automated AI and may contain errors. Do not trust its outputs without human verification.
Summary
- Reworks the demo into an authenticated editor, builder, deployer, and tester workflow.
- Adds multi-template support for Node.js, Go middleware, and Python HTTP functions.
- Adds container, publishing workflow, and Helm chart packaging for Kubernetes deployment.
- Streams build output through the backend and reflects deployment state using source hashes.
- Refreshes the React UI and removes the previous standalone code editor component.
- Main concern: API endpoints still trust
functionNamebefore using it in filesystem paths, YAML, and gateway URLs.
Approval rating (1-10)
7/10. The feature is coherent, but the backend needs stricter function-name validation before merge.
Summary per file
Summary per file
| File path | Summary |
|---|---|
.dockerignore |
Excludes build, secret, image, and markdown files from Docker context. |
.github/workflows/publish.yaml |
Adds tag-triggered multi-arch GHCR image publishing workflow. |
Dockerfile |
Adds multi-stage UI, template, and runtime image build. |
README.md |
Documents architecture, Helm install, authentication, configuration, and benchmark guidance. |
chart/function-editor/Chart.yaml |
Adds Helm chart metadata for the demo application. |
chart/function-editor/templates/NOTES.txt |
Adds post-install port-forward guidance. |
chart/function-editor/templates/_helpers.tpl |
Adds chart naming and label helper templates. |
chart/function-editor/templates/deployment.yaml |
Adds editor Deployment with env, secrets, probes, and security context. |
chart/function-editor/templates/service.yaml |
Adds ClusterIP-style service template for the editor. |
chart/function-editor/values.yaml |
Adds default image, service, config, secret, and pod settings. |
client/.gitignore |
Ignores generated templates and local secret files. |
client/auth.js |
Adds signed cookie authentication middleware and handlers. |
client/auth.test.js |
Adds Node tests for authentication configuration and session behavior. |
client/package.json |
Adds authentication tests to the existing Node test script. |
client/public/logo-text.svg |
Adds OpenFaaS logo asset for the UI. |
client/server.js |
Adds auth, templates, streamed build, deployment hash, and SPA fallback logic. |
client/src/App.css |
Updates application shell and authenticated UI styling. |
client/src/App.jsx |
Adds authentication gate, navigation shell, and shared branding. |
client/src/api.js |
Adds fetch wrapper that emits auth-required events on 401. |
client/src/components/Brand.jsx |
Adds shared OpenFaaS brand component. |
client/src/components/CodeEditor.css |
Removes obsolete standalone code editor styles. |
client/src/components/CodeEditor.jsx |
Removes obsolete standalone code editor component. |
client/src/components/FunctionEditor.jsx |
Reworks editor for templates, source files, streaming builds, and deploy state. |
client/src/components/FunctionTester.jsx |
Refreshes invocation and log viewer UI with authenticated API calls. |
client/src/components/Login.jsx |
Adds login form for editor authentication. |
client/src/dashboard-theme.css |
Adds the refreshed dashboard visual theme. |
images/function-editor.png |
Updates the README screenshot. |
Overall Assessment
The PR is a substantial and mostly cohesive rework of the demo. The packaging and UI changes fit the described workflow, and the new authentication layer is covered by focused tests. The main issue is that the backend accepts functionName from authenticated API requests and interpolates it into local filesystem paths, generated YAML, OpenFaaS service data, and gateway URLs without validating it as an OpenFaaS function name. That should be fixed before relying on these endpoints beyond the hardcoded fn1 UI path.
Detailed Review
Detailed Review
Findings
High: Validate functionName before using it in paths, YAML, and gateway requests
client/server.js:240, client/server.js:248, client/server.js:260, client/server.js:503
The new API accepts functionName from the request body and uses it directly as a YAML key, handler directory path, local filesystem path component, OpenFaaS service name, and gateway URL segment. The browser currently passes the hardcoded fn1, but the authenticated backend API is still callable directly.
For example, buildFunction() creates stack.yaml with ${functionName} and writes uploaded source files under path.join(tempDir, functionName). A name containing ../ can escape the temporary build directory when the server writes expected files, and a name containing YAML control characters can produce malformed or injected stack YAML before the request ever reaches OpenFaaS. The deploy, invoke, logs, and deployment-hash routes also use the same unvalidated value as the gateway service identifier.
Add one server-side validator at the API boundary and reject anything that is not a valid OpenFaaS function name before calling buildFunction() or proxying to the gateway. A DNS-label-style allowlist is the safest default, for example:
const functionNamePattern = /^[a-z0-9]([-a-z0-9]*[a-z0-9])?$/;
function validateFunctionName(functionName) {
return typeof functionName === 'string'
&& functionName.length <= 63
&& functionNamePattern.test(functionName);
}Use that check in /api/publish, /api/deploy, /api/invoke, /api/logs, and /api/functions/:functionName/deployment-hash, and encode URL path segments consistently when forwarding to the gateway.
Other Notes
The Helm chart and README are internally consistent about using the OpenFaaS basic-auth secret for the editor password and a separate session signing secret. The content changes are publishable after the backend validation issue is addressed.
AI agent details.
Agent processing time: 2m25.512s
Environment preparation time: 3.165s
Total time from webhook: 2m30.88s