Skip to content

Commit 5893b2d

Browse files
authored
Update SECURITY.md and CONTRIBUTING.md to address security risk of externally provided code (#8129)
* update CONTRIBUTING.md to include CLA info from discussions/3821 * update SECURITY.md to include risk of externally provided code disclaimer * remove hard line breaks for better change control
1 parent 76b22a7 commit 5893b2d

File tree

2 files changed

+45
-127
lines changed

2 files changed

+45
-127
lines changed

CONTRIBUTING.md

Lines changed: 43 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,32 @@
11
# Contributing to Open MCT
22

3-
This document describes the process of contributing to Open MCT as well
4-
as the standards that will be applied when evaluating contributions.
3+
This document describes the process of contributing to Open MCT as well as the standards that will be applied when evaluating contributions.
54

6-
Please be aware that additional agreements will be necessary before we can
7-
accept changes from external contributors.
5+
In order for external contributions to be merged, contributors must have on record a signed [Contributor License Agreement (CLA)](https://nasa.github.io/openmct/static/files/ind-cla-open-mct.pdf). More information on this process can be found [in this discussion](https://github.com/nasa/openmct/discussions/3821).
86

97
## Summary
108

119
The short version:
1210

1311
1. Write your contribution or describe your idea in the form of a [GitHub issue](https://github.com/nasa/openmct/issues/new/choose) or [start a GitHub discussion](https://github.com/nasa/openmct/discussions).
14-
2. Make sure your contribution meets code, test, and commit message
15-
standards as described below.
16-
3. Submit a pull request from a topic branch back to `master`. Include a check
17-
list, as described below. (Optionally, assign this to a specific member
18-
for review.)
19-
4. Respond to any discussion. When the reviewer decides it's ready, they
20-
will merge back `master` and fill out their own check list.
12+
2. Make sure your contribution meets code, test, and commit message standards as described below.
13+
3. Submit a pull request from a topic branch back to `master`. Include a check list, as described below. (Optionally, assign this to a specific member for review.)
14+
4. Respond to any discussion. When the reviewer decides it's ready, they will merge back `master` and fill out their own check list.
2115
5. If you are a first-time contributor, please see [this discussion](https://github.com/nasa/openmct/discussions/3821) for further information.
2216

2317
## Contribution Process
2418

25-
Open MCT uses git for software version control, and for branching and
26-
merging. The central repository is at
27-
<https://github.com/nasa/openmct.git>.
19+
Open MCT uses git for software version control, and for branching and merging. The central repository is at <https://github.com/nasa/openmct.git>.
2820

2921
### Roles
3022

31-
References to roles are made throughout this document. These are not intended
32-
to reflect titles or long-term job assignments; rather, these are used as
33-
descriptors to refer to members of the development team performing tasks in
34-
the check-in process. These roles are:
23+
References to roles are made throughout this document. These are not intended to reflect titles or long-term job assignments; rather, these are used as descriptors to refer to members of the development team performing tasks in the check-in process.
3524

36-
* _Author_: The individual who has made changes to files in the software
37-
repository, and wishes to check these in.
38-
* _Reviewer_: The individual who reviews changes to files before they are
39-
checked in.
40-
* _Integrator_: The individual who performs the task of merging these files.
41-
Usually the reviewer.
25+
These roles are:
26+
27+
* _Author_: The individual who has made changes to files in the software repository, and wishes to check these in.
28+
* _Reviewer_: The individual who reviews changes to files before they are checked in.
29+
* _Integrator_: The individual who performs the task of merging these files. Usually the reviewer.
4230

4331
### Branching
4432

@@ -48,72 +36,31 @@ Three basic types of branches may be included in the above repository:
4836
2. Topic branches
4937
3. Developer branches
5038

51-
Branches which do not fit into the above categories may be created and used
52-
during the course of development for various reasons, such as large-scale
53-
refactoring of code or implementation of complex features which may cause
54-
instability. In these exceptional cases it is the responsibility of the
55-
developer who initiates the task which motivated this branching to
56-
communicate to the team the role of these branches and any associated
57-
procedures for the duration of their use.
39+
Branches which do not fit into the above categories may be created and used during the course of development for various reasons, such as large-scale refactoring of code or implementation of complex features which may cause instability. In these exceptional cases it is the responsibility of the developer who initiates the task which motivated this branching to communicate to the team the role of these branches and any associated procedures for the duration of their use.
5840

5941
#### Master Branch
6042

61-
The role of the `master` branches is to represent the latest
62-
"ready for test" version of the software. Source code on the master
63-
branch has undergone peer review, and will undergo regular automated
64-
testing with notification on failure. Master branches may be unstable
65-
(particularly for recent features), but the intent is for the stability of
66-
any features on master branches to be non-decreasing. It is the shared
67-
responsibility of authors, reviewers, and integrators to ensure this.
43+
The role of the `master` branches is to represent the latest "ready for test" version of the software. Source code on the master branch has undergone peer review, and will undergo regular automated testing with notification on failure. Master branches may be unstable (particularly for recent features), but the intent is for the stability of any features on master branches to be non-decreasing. It is the shared responsibility of authors, reviewers, and integrators to ensure this.
6844

6945
#### Topic Branches
7046

7147
Topic branches are used by developers to perform and record work on issues.
7248

73-
Topic branches need not necessarily be stable, even when pushed to the
74-
central repository; in fact, the practice of making incremental commits
75-
while working on an issue and pushing these to the central repository is
76-
encouraged, to avoid lost work and to share work-in-progress. (Small commits
77-
also help isolate changes, which can help in identifying which change
78-
introduced a defect, particularly when that defect went unnoticed for some
79-
time, e.g. using `git bisect`.)
80-
81-
Topic branches should be named according to their corresponding issue
82-
identifiers, all lower case, without hyphens. (e.g. branch mct9 would refer
83-
to issue #9.)
84-
85-
In some cases, work on an issue may warrant the use of multiple divergent
86-
branches; for instance, when a developer wants to try more than one solution
87-
and compare them, or when a "dead end" is reached and an initial approach to
88-
resolving an issue needs to be abandoned. In these cases, a short suffix
89-
should be added to the additional branches; this may be simply a single
90-
character (e.g. wtd481b) or, where useful, a descriptive term for what
91-
distinguishes the branches (e.g. wtd481verbose). It is the responsibility of
92-
the author to communicate which branch is intended to be merged to both the
93-
reviewer and the integrator.
49+
Topic branches need not necessarily be stable, even when pushed to the central repository; in fact, the practice of making incremental commits while working on an issue and pushing these to the central repository is encouraged, to avoid lost work and to share work-in-progress. (Small commits also help isolate changes, which can help in identifying which change introduced a defect, particularly when that defect went unnoticed for some time, e.g. using `git bisect`.)
50+
51+
Topic branches should be named according to their corresponding issue identifiers, all lower case, without hyphens. (e.g. branch mct9 would refer to issue #9.)
52+
53+
In some cases, work on an issue may warrant the use of multiple divergent branches; for instance, when a developer wants to try more than one solution and compare them, or when a "dead end" is reached and an initial approach to resolving an issue needs to be abandoned. In these cases, a short suffix should be added to the additional branches; this may be simply a single character (e.g. wtd481b) or, where useful, a descriptive term for what distinguishes the branches (e.g. wtd481verbose). It is the responsibility of the author to communicate which branch is intended to be merged to both the reviewer and the integrator.
9454

9555
#### Developer Branches
9656

97-
Developer branches are any branches used for purposes outside of the scope
98-
of the above; e.g. to try things out, or maintain a "my latest stuff"
99-
branch that is not delayed by the review and integration process. These
100-
may be pushed to the central repository, and may follow any naming convention
101-
desired so long as the owner of the branch is identifiable, and so long as
102-
the name chosen could not be mistaken for a topic or master branch.
57+
Developer branches are any branches used for purposes outside of the scope of the above; e.g. to try things out, or maintain a "my latest stuff" branch that is not delayed by the review and integration process. These may be pushed to the central repository, and may follow any naming convention desired so long as the owner of the branch is identifiable, and so long as the name chosen could not be mistaken for a topic or master branch.
10358

10459
### Merging
10560

106-
When development is complete on an issue, the first step toward merging it
107-
back into the master branch is to file a Pull Request (PR). The contributions
108-
should meet code, test, and commit message standards as described below,
109-
and the pull request should include a completed author checklist, also
110-
as described below. Pull requests may be assigned to specific team
111-
members when appropriate (e.g. to draw to a specific person's attention).
61+
When development is complete on an issue, the first step toward merging it back into the master branch is to file a Pull Request (PR). The contributions should meet code, test, and commit message standards as described below, and the pull request should include a completed author checklist, also as described below. Pull requests may be assigned to specific team members when appropriate (e.g. to draw to a specific person's attention).
11262

113-
Code review should take place using discussion features within the pull
114-
request. When the reviewer is satisfied, they should add a comment to
115-
the pull request containing the reviewer checklist (from below) and complete
116-
the merge back to the master branch.
63+
Code review should take place using discussion features within the pull request. When the reviewer is satisfied, they should add a comment to the pull request containing the reviewer checklist (from below) and complete the merge back to the master branch.
11764

11865
Additionally:
11966

@@ -127,38 +74,25 @@ Additionally:
12774

12875
## Standards
12976

130-
Contributions to Open MCT are expected to meet the following standards.
131-
In addition, reviewers should use general discretion before accepting
132-
changes.
77+
Contributions to Open MCT are expected to meet the following standards. In addition, reviewers should use general discretion before accepting changes.
13378

13479
### Code Standards
13580

136-
JavaScript sources in Open MCT must satisfy the [ESLint](https://eslint.org/) rules defined in
137-
this repository. [Prettier](https://prettier.io/) is used in conjunction with ESLint to enforce code style
138-
via automated formatting. These are verified by the command line build.
81+
JavaScript sources in Open MCT must satisfy the [ESLint](https://eslint.org/) rules defined in this repository. [Prettier](https://prettier.io/) is used in conjunction with ESLint to enforce code style via automated formatting. These are verified by the command line build.
13982

14083
#### Code Guidelines
14184

14285
The following guidelines are provided for anyone contributing source code to the Open MCT project:
14386

14487
1. Write clean code. Here’s a good summary - <https://github.com/ryanmcdermott/clean-code-javascript>.
14588
1. Include JSDoc for any exposed API (e.g. public methods, classes).
146-
1. Include non-JSDoc comments as-needed for explaining private variables,
147-
methods, or algorithms when they are non-obvious. Otherwise code
148-
should be self-documenting.
149-
1. Classes and Vue components should use camel case, first letter capitalized
150-
(e.g. SomeClassName).
151-
1. Methods, variables, fields, events, and function names should use camelCase,
152-
first letter lower-case (e.g. someVariableName).
89+
1. Include non-JSDoc comments as-needed for explaining private variables, methods, or algorithms when they are non-obvious. Otherwise code should be self-documenting.
90+
1. Classes and Vue components should use camel case, first letter capitalized (e.g. SomeClassName).
91+
1. Methods, variables, fields, events, and function names should use camelCase, first letter lower-case (e.g. someVariableName).
15392
1. Source files that export functions should use camelCase, first letter lower-case (eg. testTools.js)
154-
1. Constants (variables or fields which are meant to be declared and
155-
initialized statically, and never changed) should use only capital
156-
letters, with underscores between words (e.g. SOME_CONSTANT). They should always be declared as `const`s
157-
1. File names should be the name of the exported class, plus a .js extension
158-
(e.g. SomeClassName.js).
159-
1. Avoid anonymous functions, except when functions are short (one or two lines)
160-
and their inclusion makes sense within the flow of the code
161-
(e.g. as arguments to a forEach call). Anonymous functions should always be arrow functions.
93+
1. Constants (variables or fields which are meant to be declared and initialized statically, and never changed) should use only capital letters, with underscores between words (e.g. SOME_CONSTANT). They should always be declared as `const`s
94+
1. File names should be the name of the exported class, plus a .js extension (e.g. SomeClassName.js).
95+
1. Avoid anonymous functions, except when functions are short (one or two lines) and their inclusion makes sense within the flow of the code (e.g. as arguments to a forEach call). Anonymous functions should always be arrow functions.
16296
1. Named functions are preferred over functions assigned to variables.
16397
eg.
16498

@@ -176,13 +110,10 @@ The following guidelines are provided for anyone contributing source code to the
176110
}
177111
```
178112

179-
1. Avoid deep nesting (especially of functions), except where necessary
180-
(e.g. due to closure scope).
113+
1. Avoid deep nesting (especially of functions), except where necessary (e.g. due to closure scope).
181114
1. End with a single new-line character.
182-
1. Always use ES6 `Class`es and inheritance rather than the pre-ES6 prototypal
183-
pattern.
184-
1. Within a given function's scope, do not mix declarations and imperative
185-
code, and present these in the following order:
115+
1. Always use ES6 `Class`es and inheritance rather than the pre-ES6 prototypal pattern.
116+
1. Within a given function's scope, do not mix declarations and imperative code, and present these in the following order:
186117
* First, variable declarations and initialization.
187118
* Secondly, imperative statements.
188119
* Finally, the returned value. A single return statement at the end of the function should be used, except where an early return would improve code clarity.
@@ -233,41 +164,28 @@ The following guidelines are provided for anyone contributing source code to the
233164
pluginSpec.js
234165
```
235166

236-
Deviations from Open MCT code style guidelines require two-party agreement,
237-
typically from the author of the change and its reviewer.
167+
Deviations from Open MCT code style guidelines require two-party agreement, typically from the author of the change and its reviewer.
238168

239169
### Commit Message Standards
240170

241171
Commit messages should:
242172

243-
* Contain a one-line subject, followed by one line of white space,
244-
followed by one or more descriptive paragraphs, each separated by one
245-
 line of white space.
246-
* Contain a short (usually one word) reference to the feature or subsystem
247-
the commit effects, in square brackets, at the start of the subject line
248-
(e.g. `[Documentation] Draft of check-in process`).
173+
* Contain a one-line subject, followed by one line of white space, followed by one or more descriptive paragraphs, each separated by one line of white space.
174+
* Contain a short (usually one word) reference to the feature or subsystem the commit effects, in square brackets, at the start of the subject line (e.g. `[Documentation] Draft of check-in process`).
249175
* Contain a reference to a relevant issue number in the body of the commit.
250-
* This is important for traceability; while branch names also provide this,
251-
you cannot tell from looking at a commit what branch it was authored on.
252-
* This may be omitted if the relevant issue is otherwise obvious from the
253-
commit history (that is, if using `git log` from the relevant commit
254-
directly leads to a similar issue reference) to minimize clutter.
176+
* This is important for traceability; while branch names also provide this, you cannot tell from looking at a commit what branch it was authored on.
177+
* This may be omitted if the relevant issue is otherwise obvious from the commit history (that is, if using `git log` from the relevant commit directly leads to a similar issue reference) to minimize clutter.
255178
* Describe the change that was made, and any useful rationale therefore.
256-
* Comments in code should explain what things do, commit messages describe
257-
how they came to be done that way.
258-
* Provide sufficient information for a reviewer to understand the changes
259-
made and their relationship to previous code.
179+
* Comments in code should explain what things do, commit messages describe how they came to be done that way.
180+
* Provide sufficient information for a reviewer to understand the changes made and their relationship to previous code.
260181

261182
Commit messages should not:
262183

263184
* Exceed 54 characters in length on the subject line.
264185
* Exceed 72 characters in length in the body of the commit,
265-
* Except where necessary to maintain the structure of machine-readable or
266-
machine-generated text (e.g. error messages).
186+
* Except where necessary to maintain the structure of machine-readable or machine-generated text (e.g. error messages).
267187

268-
See [Contributing to a Project](http://git-scm.com/book/ch5-2.html) from
269-
Pro Git by Shawn Chacon and Ben Straub for a bit of the rationale behind
270-
these standards.
188+
See [Contributing to a Project](http://git-scm.com/book/ch5-2.html) from Pro Git by Shawn Chacon and Ben Straub for a bit of the rationale behind these standards.
271189

272190
## Issue Reporting
273191

@@ -282,8 +200,6 @@ Issue severity is categorized as follows (in ascending order):
282200

283201
## Check Lists
284202

285-
The following check lists should be completed and attached to pull requests
286-
when they are filed (author checklist) and when they are merged (reviewer
287-
checklist).
203+
The following check lists should be completed and attached to pull requests when they are filed (author checklist) and when they are merged (reviewer checklist).
288204

289205
[Within PR Template](.github/PULL_REQUEST_TEMPLATE.md)

SECURITY.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Security Policy
22

3+
Open MCT is an open source project and may contain externally provided code. External contributions must follow the guidelines in [CONTRIBUTING.md](CONTRIBUTING.md).
4+
35
The Open MCT team secures our code base using a combination of code review, dependency review, and periodic security reviews. Static analysis performed during automated verification additionally safeguards against common coding errors which may result in vulnerabilities.
46

57
### Reporting a Vulnerability

0 commit comments

Comments
 (0)