Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Asset names with invalid filename characters do not work well with .ckl archive exports #266

Closed
cd-rite opened this issue Apr 24, 2021 · 2 comments
Closed

Asset names with invalid filename characters do not work well with .ckl archive exports #266

cd-rite opened this issue Apr 24, 2021 · 2 comments
Assignees
@cd-rite
Labels
API bug Something isn't working good first issue Good for newcomers UI

Comments

@cd-rite
Copy link
Collaborator

cd-rite commented Apr 24, 2021 •
edited
Loading

current behavior:
Asset names that have slashes in them will create a new folder in the zip.
Asset names with ? do not appear in the .zip at all.

Apply filtering/substitution to Asset names (either at create/modify time in UI or upon archive file creation) to be sure files that are generated based on those names are valid.

This substitution seems to be already happening in the Asset Review individual .ckl export function.
@cd-rite cd-rite added bug Something isn't working good first issue Good for newcomers API UI labels Apr 24, 2021
@cd-rite cd-rite self-assigned this Apr 24, 2021
@cd-rite cd-rite changed the title Substitute invalid file name characters when .ckl archive exports Asset names with invalid filename characters do not work well with .ckl archive exports Apr 24, 2021
@csmig
Copy link
Member

csmig commented Apr 24, 2021 •
edited
Loading

This issue affects GET​/collections​/{collectionId}​/poam as well, the Collection name could be unsafe in a filename.

I recommend addressing this in /api/source/utils/writer.js. There are several methods there that write an inline file response, whose implementations differ only on the Content-Type. Would be good to refactor these multiple convenience methods so they each call a common writeInlineFile method with the desired Content-Type.

The new method could use the filenamify modue to transform the proposed filename to a safe name. But maybe a regex could do it, too.

@csmig
Copy link
Member

csmig commented Apr 24, 2021

@cd-rite I just noticed you assigned this to yourself. I wrote that last message for my future-self, so feel free to take the recommendations or not!

@csmig csmig mentioned this issue Apr 27, 2021
Closed
csmig added a commit to csmig/stig-manager that referenced this issue Apr 27, 2021
@csmig
fix NUWCDIVNPT#266: sanitize exported filenames
383214e
csmig added a commit to csmig/stig-manager that referenced this issue Apr 27, 2021
@csmig
fix NUWCDIVNPT#266: sanitize exported filenames
d01fac5
@csmig csmig closed this as completed in ab8800b Apr 27, 2021
cd-rite added a commit to cd-rite/stig-manager that referenced this issue May 6, 2021
@cd-rite @csmig @dependabot
Tests 5 5 (#20)
0c04320 
* feat: Collection export management (NUWCDIVNPT#169)

* fix: case-insensitive filename matching (NUWCDIVNPT#192)

* fix: Improved output when importing STIG XML (NUWCDIVNPT#192)

* doc: Show Export CKLs in screenshots

* chore: Bump release to 1.0.0-beta.22

* adjust path to docker readme (NUWCDIVNPT#196)

* doc: Added some documentation about new .ckl archive export feature. (NUWCDIVNPT#203)

* removed some todos

* stig archive export feature

* feat: name-match params and duplicate handling (NUWCDIVNPT#204)

* feat: case-sensitive collation for benchmarkId in MySQL (NUWCDIVNPT#206)

* Common tasks elaboration, other edits (NUWCDIVNPT#208)

* feat: progress bar styling (NUWCDIVNPT#209)

* feat: UI shows collectionId (NUWCDIVNPT#210)

* fix: remove hard-coded reference to schema (NUWCDIVNPT#211)

* chore: Bump release to 1.0.0-beta.23

* fix: reduce deadlock potential (NUWCDIVNPT#216)

* api links (NUWCDIVNPT#219)

* build(deps): bump y18n from 3.2.1 to 3.2.2 in /api/source

Bumps [y18n](https://github.com/yargs/y18n) from 3.2.1 to 3.2.2.
- [Release notes](https://github.com/yargs/y18n/releases)
- [Changelog](https://github.com/yargs/y18n/blob/master/CHANGELOG.md)
- [Commits](https://github.com/yargs/y18n/commits)

Signed-off-by: dependabot[bot] <support@github.com>

* doc: Added a little more about .ckl and data handling (NUWCDIVNPT#223)

* just rst changes

* sphinx generation

* fix: Exports on multiple reports (NUWCDIVNPT#224)

* Multiple fixes and features (NUWCDIVNPT#225)

* feat: return 401 when no token provided

* feat: home-widget-bwrap

* fix: collectionReview buttons

* fix: deadlock prevention status updates

* chore: Bump release to 1.0.0-beta.24

* fix: fetch STIG/SCAP if configured at bootstrap (NUWCDIVNPT#227)

Fixes NUWCDIVNPT#213

* Multiple fix and features (NUWCDIVNPT#228)

* feat: CKL parser retains empty comments
* feat: enable accept when selections include accept
* fix: review form button behaviors, etc. (NUWCDIVNPT#215)

* chore: remove unused oracledb dependency (NUWCDIVNPT#229)

* chore: remove unused oracledb dependency

* Remove unused require

* chore: Bump release to 1.0.0-beta.25

* feat: Manage Assets -> multi-delete (NUWCDIVNPT#232), columns (NUWCDIVNPT#236)

* fix: include promisfied confirm (NUWCDIVNPT#237)

* build(deps): bump urllib3 from 1.26.3 to 1.26.4 in /docs (NUWCDIVNPT#238)

Bumps [urllib3](https://github.com/urllib3/urllib3) from 1.26.3 to 1.26.4.
- [Release notes](https://github.com/urllib3/urllib3/releases)
- [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst)
- [Commits](urllib3/urllib3@1.26.3...1.26.4)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* doc: updates regarding ckl -> stigman field mappings, clients folder when running from source (NUWCDIVNPT#241)

doc: updates regarding ckl -> stigman field mappings, clients folder when running from source

* feat: Tooltips for Review labels and headers (NUWCDIVNPT#240) (NUWCDIVNPT#242)

* feat: mercury-medium color is more blue (NUWCDIVNPT#243)

* fix: sticky bit for world-writable dirs created by npm (NUWCDIVNPT#245)

* chore: Bump release to 1.0.0-beta.26

* fix: increased length of asset name,ip,mac,fqdn and allow more nulls  (NUWCDIVNPT#251)

* added maxLength properties of 255 for ip, mac, asset name, and fqdn; added nullable:true for collection description properties

* removed vtype specification for ip address, as we will no longer be validating ip address field.

* Added migration file to alter varchar sizes for asset ip, mac, and name

* fix: batch import continues on error, refreshes grids (NUWCDIVNPT#252)

* feat: Ext.LoadMask looks for store.smMaskDelay (NUWCDIVNPT#254)

* chore: Bump release to 1.0.0-beta.27

* fix: log servicename if present (NUWCDIVNPT#198)

* fix: Attach => Assign STIG (NUWCDIVNPT#118)

* fix: response schema for /opt/configuration (NUWCDIVNPT#147)

* fix: create date is not ISO8601 UTC (NUWCDIVNPT#189)

* fix: handle property chains with hyphens (NUWCDIVNPT#257)

* fix: cast userId as char (NUWCDIVNPT#249)

* feat: format roles claim with bracket notation and optional chaining (NUWCDIVNPT#190)

* fix: SET NAME to utf8mb4 encoding (NUWCDIVNPT#262)

* fix: New/Delete => Assign/Unassign (NUWCDIVNPT#261)

* fix: New/Delete => Assign/Unassign (NUWCDIVNPT#118)

* dump docker logs on failure or cancellation

* fix: Filter members only on .xml extension  (NUWCDIVNPT#260)

* Removed attempts to filter STIG processing based on filename, since they do not seem to follow any reliable convention. Restricted error responses to just parser error message (removed stack trace portion so it does not show up in user's import log).

* added specific try/catch blocks around xml parsing

* fix NUWCDIVNPT#264: Display feedback for rejected reviews (NUWCDIVNPT#265)

* chore: Bump release to 1.0.0-beta.28

* fix NUWCDIVNPT#256: CKL site/instance handling; UI refactor (NUWCDIVNPT#268)

* chore: Bump release to 1.0.0-beta.29

* ironbank => development sign+image

* fix NUWCDIVNPT#266: sanitize exported filenames (NUWCDIVNPT#273)

* fix NUWCDIVNPT#270: ROLE element default value 'None' (NUWCDIVNPT#272)

* chore: Bump release to 1.0.0-beta.30

* fix NUWCDIVNPT#276: remove reference to database 'stigman'

* chore: remove obsolete docker dir (NUWCDIVNPT#278)

* Docs: Added default_group to prevent guid generation, removed doctrees, added a bit of info to Contributing doc. (NUWCDIVNPT#281)

* added default_group for images to stop guid generation
* removed doctrees
* added doctrees to .gitignore
*  added a couple paragraphs to contributing doc

* Endpoint updates (NUWCDIVNPT#284)

* feat: GET /assets metadata parameter

* feat: PUT /assets/{assetId}/stigs/{benchmarkId}

* tests match OpenAPI spec

* fix NUWCDIVNPT#145: Review vetting for all users (NUWCDIVNPT#285)

* fix NUWCDIVNPT#145: Review vetting for lvl1 users

* lvl1 cross-boundary tests, xccdf test file added, workflow updated to run new folder. Removed extra folders from Collection

* refactor adminStats, scc parser, tests, workflow

Co-authored-by: cd-rite <github-rite@notdoneyet.net>

* feat: Drag from Review History (NUWCDIVNPT#288)

* fix NUWCDIVNPT#275: handle rule-result without check (NUWCDIVNPT#290)

* fix NUWCDIVNPT#275: handle rule-result without check

* asset properties and benchmarkId check

* chore: Bump release to 1.0.0-beta.31

* checks for asset with no assigned STIGs, changed lvl1 checks to look for existing rule to which it does not have access (as opposed to non-existent rule)

* marked tests as continue-on-error so remaining tests would still run.

Co-authored-by: csmig <carlsmigielski@gmail.com>
Co-authored-by: csmig <33138761+csmig@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cd-rite cd-rite

Labels
API bug Something isn't working good first issue Good for newcomers UI
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@csmig @cd-rite