Skip to content

Commit 98155d0

Browse files
jankeromnesgtsiolis
andcommitted
Address code review feedback
Co-authored-by: George Tsiolis <[email protected]>
1 parent 40b579a commit 98155d0

File tree

2 files changed

+17
-13
lines changed

2 files changed

+17
-13
lines changed

components/dashboard/src/projects/ProjectVariables.tsx

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,29 +44,27 @@ export default function () {
4444
<div className="mb-2 flex">
4545
<div className="flex-grow">
4646
<h3>Environment Variables</h3>
47-
<h2 className="text-gray-500">Manage environment variables for your project.</h2>
47+
<h2 className="text-gray-500">Manage project-specific environment variables.</h2>
4848
</div>
49-
{envVars.length > 0 && <button onClick={() => setShowAddVariableModal(true)}>Add Variable</button>}
49+
{envVars.length > 0 && <button onClick={() => setShowAddVariableModal(true)}>New Variable</button>}
5050
</div>
5151
{envVars.length === 0
5252
? <div className="bg-gray-100 dark:bg-gray-800 rounded-xl w-full py-28 flex flex-col items-center">
5353
<h3 className="text-center pb-3 text-gray-500 dark:text-gray-400">No Environment Variables</h3>
54-
<p className="text-center pb-6 text-gray-500 text-base w-96">Here you can set <strong>project-specific environment variables</strong> that will be visible during prebuilds and (optionally) in workspaces for this project.</p>
54+
<p className="text-center pb-6 text-gray-500 text-base w-96">All <strong>project-specific environment variables</strong> will be visible in prebuilds and optionally in workspaces for this project.</p>
5555
<button onClick={() => setShowAddVariableModal(true)}>New Variable</button>
5656
</div>
5757
: <>
5858
<ItemsList>
59-
<Item header={true} className="grid grid-cols-4 items-center">
59+
<Item header={true} className="grid grid-cols-3 items-center">
6060
<ItemField>Name</ItemField>
61-
<ItemField>Value</ItemField>
62-
<ItemField>Visible in Workspaces?</ItemField>
61+
<ItemField>Hidden in Workspaces</ItemField>
6362
<ItemField></ItemField>
6463
</Item>
6564
{envVars.map(variable => {
66-
return <Item className="grid grid-cols-4 items-center">
65+
return <Item className="grid grid-cols-3 items-center">
6766
<ItemField>{variable.name}</ItemField>
68-
<ItemField>****</ItemField>
69-
<ItemField>{variable.censored ? 'Hidden' : 'Visible'}</ItemField>
67+
<ItemField>{variable.censored ? 'Yes' : 'No'}</ItemField>
7068
<ItemField className="flex justify-end">
7169
<ItemFieldContextMenu menuEntries={[
7270
{
@@ -103,9 +101,9 @@ function AddVariableModal(props: { project?: Project, onClose: () => void }) {
103101
}
104102

105103
return <Modal visible={true} onClose={props.onClose} onEnter={() => { addVariable(); return false; }}>
106-
<h3 className="mb-4">Add Variable</h3>
104+
<h3 className="mb-4">New Variable</h3>
107105
<div className="border-t border-b border-gray-200 dark:border-gray-800 -mx-6 px-6 py-4 flex flex-col">
108-
<AlertBox><strong>Project variables might be accessible by anyone with read access to your repository.</strong><br/>Secret values can be exposed if they are printed in the logs, persisted to the file system, or made visible in workspaces.</AlertBox>
106+
<AlertBox><strong>Project-specific environment variables might be accessible by anyone with read access to your repository.</strong><br/>Secret values can be exposed if they are printed in the logs, persisted to the file system, or made visible in workspaces.</AlertBox>
109107
{error && <div className="bg-gitpod-kumquat-light rounded-md p-3 text-gitpod-red text-sm mb-2">
110108
{error}
111109
</div>}
@@ -118,7 +116,7 @@ function AddVariableModal(props: { project?: Project, onClose: () => void }) {
118116
<input className="w-full" type="text" name="value" value={value} onChange={e => setValue(e.target.value)} />
119117
</div>
120118
<div className="mt-4">
121-
<CheckBox title="Hide in Workspaces" desc="Project variables are visible during prebuilds. Choose whether this variable should be visible in workspaces as well." checked={censored} onChange={() => setCensored(!censored)} />
119+
<CheckBox title="Hide in Workspaces" desc="Project-specific environment variables are visible in prebuilds. Choose whether this variable should be visible in workspaces as well." checked={censored} onChange={() => setCensored(!censored)} />
122120
</div>
123121
{!censored && <div className="mt-4">
124122
<InfoBox>This value will be directly visible to anyone who can open your repository in Gitpod.</InfoBox>

components/gitpod-db/src/typeorm/project-db-impl.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,13 @@ export class ProjectDBImpl implements ProjectDB {
108108
}
109109
}
110110

111-
public async setProjectEnvironmentVariable(projectId: string, name: string, value: string, censored: boolean): Promise<void>{
111+
public async setProjectEnvironmentVariable(projectId: string, name: string, value: string, censored: boolean): Promise<void> {
112+
if (!name) {
113+
throw new Error('Variable name cannot be empty');
114+
}
115+
if (!/[a-zA-Z_][a-zA-Z0-9_]*/.test(name)) {
116+
throw new Error('Please choose a variable name containing only letters, numbers, or _, and doesn\'t start with numbers');
117+
}
112118
const envVarRepo = await this.getProjectEnvVarRepo();
113119
const envVarWithValue = await envVarRepo.findOne({ projectId, name, deleted: false });
114120
if (envVarWithValue) {

0 commit comments

Comments
 (0)