diff --git a/.editorconfig b/.editorconfig
index 47ae637..77cadaa 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -10,6 +10,13 @@ indent_style = space
insert_final_newline = true
trim_trailing_whitespace = true
+[*.md]
+trim_trailing_whitespace = false
+
+[*{.yml,.js}]
+indent_size = 2
+indent_style = space
+
[{*.yml,package.json}]
indent_size = 2
diff --git a/composer.json b/composer.json
index db95b4a..9b44349 100644
--- a/composer.json
+++ b/composer.json
@@ -19,7 +19,7 @@
],
"require": {
"php": ">=5.6.0",
- "silverstripe/framework": "^3",
+ "silverstripe/framework": "^3.2",
"sensiolabs/security-checker": "^4",
"symbiote/silverstripe-queuedjobs": "^2"
},
@@ -27,7 +27,7 @@
"spekulatius/silverstripe-composer-security-checker": "self.version"
},
"suggest": {
- "friendsofsilverstripe/silverstripe-maintenance": "Display advisory information via Site Summary Report",
+ "bringyourownideas/silverstripe-maintenance": "Display advisory information via Site Summary Report",
"roave/security-advisories": "Raises a conflict on attempt to install a vulnerable module"
},
"support": {
diff --git a/css/securityalerts.css b/css/securityalerts.css
new file mode 100644
index 0000000..aa40e9d
--- /dev/null
+++ b/css/securityalerts.css
@@ -0,0 +1,7 @@
+.package-summary__security-alerts {
+ margin-top: 8px;
+}
+
+.security-alerts__list {
+ display: none;
+}
diff --git a/javascript/summaryalerts.js b/javascript/summaryalerts.js
new file mode 100644
index 0000000..1d830f7
--- /dev/null
+++ b/javascript/summaryalerts.js
@@ -0,0 +1,29 @@
+(function($) {
+ $.entwine('ss', function($) {
+ $('.package-summary__security-alerts').entwine({
+ IsShown: false,
+ onclick: function(event) {
+ if ($(event.target).is('strong, strong>span')) {
+ this.toggleSecurityNotices();
+ }
+ },
+ toggleSecurityNotices: function() {
+ if (this.getIsShown()) {
+ this.hideSecurityNotices();
+ } else {
+ this.showSecurityNotices();
+ }
+ },
+ showSecurityNotices: function() {
+ this.children('dl').show();
+ this.find('strong>span').text('Hide');
+ this.setIsShown(true);
+ },
+ hideSecurityNotices: function() {
+ this.children('dl').hide();
+ this.find('strong>span').text('Show');
+ this.setIsShown(false);
+ }
+ });
+ });
+})(jQuery)
diff --git a/src/Extensions/PackageSecruityExtension.php b/src/Extensions/PackageSecruityExtension.php
index 78712bc..55493fc 100644
--- a/src/Extensions/PackageSecruityExtension.php
+++ b/src/Extensions/PackageSecruityExtension.php
@@ -10,16 +10,48 @@ class PackageSecurityExtension extends DataExtension
'listCVEs' => 'Security alerts',
];
+ /**
+ * Simply returns a comma separated list of active CVE numbers for this record.
+ * Used in CSV exports as a type of brief indication (as opposed to full info)
+ */
public function listCVEs()
{
$alerts = $this->owner->SecurityAlerts()->Column('CVE');
return $alerts ? implode(', ', $alerts) : null;
}
+ /**
+ * Renders the SecurityAlerts relationship
+ * intended for use in the on screen version of the SiteSummary Report.
+ */
public function listAlerts()
{
$templates = ['PackageSecurityAlerts'];
$this->owner->extend('updateListAlerts', $templates);
return $this->owner->renderWith($templates);
}
+
+ /**
+ * updates the badges that render as part of the screen targeted
+ * summary for this Package
+ *
+ * @param array $badges in the format of [title => type]
+ */
+ public function updateBadges(&$badges)
+ {
+ if ($this->owner->SecurityAlerts()->exists()) {
+ $badges['RISK: Security'] = 'warning';
+ }
+ }
+
+ /**
+ * Appends our own summary info to that of the default output
+ * of the Package getSummary method.
+ *
+ * @param HTMLText $summary
+ */
+ public function updateSummary(&$summary)
+ {
+ $summary->setValue($summary . $this->listAlerts());
+ }
}
diff --git a/src/Extensions/SiteSummaryExtension.php b/src/Extensions/SiteSummaryExtension.php
index 9224300..7aac9b3 100644
--- a/src/Extensions/SiteSummaryExtension.php
+++ b/src/Extensions/SiteSummaryExtension.php
@@ -2,14 +2,31 @@
class SiteSummaryExtension extends Extension
{
+ /**
+ * Update screen bound report columns to remove the text (csv) column
+ * listing CVE numbers, and include the view assets to render appropriately
+ *
+ * @param array $columns Report display columns
+ */
public function updateColumns(&$columns)
{
+ Requirements::css('silverstripe-composer-security-checker/css/securityalerts.css');
+ Requirements::javascript('silverstripe-composer-security-checker/javascript/summaryalerts.js');
unset($columns['listCVEs']);
}
- public function updateCMSFields(&$fields)
+ /**
+ * Update the Package's screen bound summary info with little badges to indicate
+ * security alerts are present for this package
+ *
+ * @param array $alerts a list of alerts to display
+ */
+ public function updateAlerts(&$alerts)
{
- $summaryInfo = $this->owner->sourceRecords()->renderWith('PackageSecurityAlerts');
- $fields->unshift(LiteralField::create('AlertSummary', $summaryInfo));
+ $securityWarnings = $this->owner->sourceRecords()->filter('SecurityAlerts.ID:GreaterThan', 0);
+
+ if ($securityWarnings->exists()) {
+ $alerts['SecurityAlerts'] = $securityWarnings->renderWith('SecurityAlertSummary');
+ }
}
}
diff --git a/src/Jobs/CVECheckJob.php b/src/Jobs/CVECheckJob.php
index bb8b0e1..d6c8492 100644
--- a/src/Jobs/CVECheckJob.php
+++ b/src/Jobs/CVECheckJob.php
@@ -19,16 +19,16 @@ class CVECheckJob extends AbstractQueuedJob implements QueuedJob
/**
* @return CVECheckTask
*/
- public function getCVECheckTask()
+ public function getCheckTask()
{
return $this->checkTask;
}
/**
- * @param CVECheckTask
+ * @param CVECheckTask $checkTask
* @return CVECheckJob
*/
- public function setCVECheckTask($checkTask)
+ public function setCheckTask(CVECheckTask $checkTask)
{
$this->checkTask = $checkTask;
return $this;
@@ -52,7 +52,7 @@ public function getJobType()
public function process()
{
// run the task
- $task = $this->getCVECheckTask();
+ $task = $this->getCheckTask();
$task->run(null);
// mark job as completed
diff --git a/src/Tasks/CVECheckTask.php b/src/Tasks/CVECheckTask.php
index 0e95b45..defdd5d 100644
--- a/src/Tasks/CVECheckTask.php
+++ b/src/Tasks/CVECheckTask.php
@@ -31,9 +31,9 @@ public function getSecurityChecker()
/**
* @param SecurityChecker $securityChecker
- * @return CVECheckTask $this
+ * @return $this
*/
- public function setSecurityChecker($securityChecker)
+ public function setSecurityChecker(SecurityChecker $securityChecker)
{
$this->securityChecker = $securityChecker;
return $this;
@@ -61,7 +61,7 @@ public function run($request)
// Is this vulnerability known? No, lets add it.
if ((int) $vulnerability->count() === 0) {
- $vulnerability = new CVE();
+ $vulnerability = CVE::create();
$vulnerability->PackageName = $package;
$vulnerability->Version = $packageDetails['version'];
$vulnerability->Title = $details['title'];
@@ -87,15 +87,15 @@ public function run($request)
}
// remove all entries which are resolved (no longer $validEntries)
- $removeOldCVEs = SQLDelete::create('CVE');
+ $removeOldCVEs = SQLDelete::create('"CVE"');
if (empty($validEntries)) {
- // There were no CVEs listed for our installation
+ // There were no CVEs listed for our installation - so flush any old data
$removeOldCVEs->execute();
} else {
$removable = CVE::get()->exclude(array('ID' => $validEntries));
if ($removable->exists()) {
// Be careful not to remove all CVEs on the case that entry is valid
- $removeOldCVEs = $removeOldCVEs->addWhere('ID', $removable->column('ID'));
+ $removeOldCVEs = $removeOldCVEs->addWhere('"ID"', $removable->column('ID'));
$removeOldCVEs->execute();
}
}
diff --git a/templates/PackageSecurityAlerts.ss b/templates/PackageSecurityAlerts.ss
index 170e708..8e1ee49 100644
--- a/templates/PackageSecurityAlerts.ss
+++ b/templates/PackageSecurityAlerts.ss
@@ -1,13 +1,11 @@
-<% loop $Me %>
- <% if $SecurityAlerts %>
-
-
$Name
-
- <% loop $SecurityAlerts %>
- - $CVE
- - $Title
- <% end_loop %>
-
-
+
View security alert info
+
+ <% loop $SecurityAlerts %>
+ - $CVE
+ - $Title
+ <% end_loop %>
+
+
+ Security alert
+ <% if Count > 1 %>Notices have<% else %>A notice has<% end_if %> been issued for $Count of your modules. Review and updating is recommended.
+
\ No newline at end of file
diff --git a/tests/CVECheckTaskTest.php b/tests/CVECheckTaskTest.php
index f73a10f..dbd576a 100644
--- a/tests/CVECheckTaskTest.php
+++ b/tests/CVECheckTaskTest.php
@@ -41,12 +41,12 @@ public function testUpdatesAreSaved()
$checkTask->setSecurityChecker($securityCheckerMock);
$preCheck = CVE::get();
- $this->assertEquals(0, $preCheck->count(), 'database is empty to begin with');
+ $this->assertCount(0, $preCheck, 'database is empty to begin with');
$checkTask->run(null);
$postCheck = CVE::get();
- $this->assertEquals(1, $postCheck->count(), 'CVE has been stored');
+ $this->assertCount(1, $postCheck, 'CVE has been stored');
}
public function testNoDuplicates()
@@ -56,17 +56,17 @@ public function testNoDuplicates()
$checkTask->setSecurityChecker($securityCheckerMock);
$preCheck = CVE::get();
- $this->assertEquals(0, $preCheck->count(), 'database is empty to begin with');
+ $this->assertCount(0, $preCheck, 'database is empty to begin with');
$checkTask->run(null);
$postCheck = CVE::get();
- $this->assertEquals(1, $postCheck->count(), 'CVE has been stored');
+ $this->assertCount(1, $postCheck, 'CVE has been stored');
$checkTask->run(null);
$postCheck = CVE::get();
- $this->assertEquals(1, $postCheck->count(), 'The CVE isn\'t stored twice.');
+ $this->assertCount(1, $postCheck, 'The CVE isn\'t stored twice.');
}
public function testCVERemovals()
@@ -78,7 +78,7 @@ public function testCVERemovals()
$checkTask->run(null);
$preCheck = CVE::get();
- $this->assertEquals(1, $preCheck->count(), 'database has stored CVEs');
+ $this->assertCount(1, $preCheck, 'database has stored CVEs');
$securityCheckerMock = $this->getSecurityCheckerMock(true);
$checkTask->setSecurityChecker($securityCheckerMock);
@@ -86,6 +86,6 @@ public function testCVERemovals()
$checkTask->run(null);
$postCheck = CVE::get();
- $this->assertEquals(0, $postCheck->count(), 'database is empty to finish with');
+ $this->assertCount(0, $postCheck, 'database is empty to finish with');
}
}