From b4ec5f53669970b71a0615fe38cac49310387976 Mon Sep 17 00:00:00 2001 From: Rachal Cassity Date: Mon, 29 Apr 2024 16:14:10 -0500 Subject: [PATCH] Revert "ClamAV in Containers (#15965)" This reverts commit 0857270b877b7a264c2d6fd456b7c931270b1770. --- .github/CODEOWNERS | 15 +- .github/workflows/audit_service_tags.yml | 11 +- .github/workflows/code_checks.yml | 11 +- .gitignore | 4 - Dockerfile | 139 +++++++++++------- Gemfile | 2 +- Gemfile.lock | 8 +- Makefile | 19 ++- Procfile | 2 + app/uploaders/uploader_virus_scan.rb | 7 +- bin/fake_clamdscan | 5 + clamav_tmp/.keep | 0 config/clamd.conf | 8 +- config/freshclam.conf | 2 +- config/initializers/clamav.rb | 6 - config/initializers/clamscan.rb | 5 + config/settings.local.yml.example | 8 +- config/settings.yml | 7 - docker-compose-clamav.yml | 10 -- docker-compose-deps.yml | 9 +- docker-compose.review.yml | 99 ++++++------- docker-compose.test.yml | 3 +- docker-compose.yml | 84 +++++------ lib/clamav/commands/patch_scan_command.rb | 43 ------ lib/clamav/patch_client.rb | 76 ---------- lib/common/file_helpers.rb | 17 --- lib/common/virus_scan.rb | 16 +- lib/shrine/plugins/validate_virus_free.rb | 8 +- .../plugins/validate_virus_free_spec.rb | 40 +++-- .../dependency_claim_spec.rb | 7 +- .../persistent_attachments/lgy_claim_spec.rb | 7 +- .../pension_burial_spec.rb | 7 +- spec/requests/claim_documents_spec.rb | 4 +- spec/simplecov_helper.rb | 1 + spec/spec_helper.rb | 1 + spec/support/uploader_helpers.rb | 8 +- spec/uploaders/uploader_virus_scan_spec.rb | 10 +- 37 files changed, 281 insertions(+), 428 deletions(-) create mode 100755 bin/fake_clamdscan delete mode 100644 clamav_tmp/.keep delete mode 100644 config/initializers/clamav.rb create mode 100644 config/initializers/clamscan.rb delete mode 100644 docker-compose-clamav.yml delete mode 100644 lib/clamav/commands/patch_scan_command.rb delete mode 100644 lib/clamav/patch_client.rb diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 8979c9724a5..0bcaba3a057 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -6,16 +6,11 @@ Dangerfile @department-of-veterans-affairs/backend-review-group Dockerfile @department-of-veterans-affairs/backend-review-group Dockerfile-k8s @department-of-veterans-affairs/backend-review-group -docker-compose.yml @department-of-veterans-affairs/backend-review-group -docker-compose-clamav.yml @department-of-veterans-affairs/backend-review-group -docker-compose-deps.yml @department-of-veterans-affairs/backend-review-group -docker-compose.review.yml @department-of-veterans-affairs/backend-review-group -docker-compose.test.yml @department-of-veterans-affairs/backend-review-group +docker-compose* @department-of-veterans-affairs/backend-review-group Gemfile @department-of-veterans-affairs/backend-review-group Gemfile.lock @department-of-veterans-affairs/backend-review-group Jenkinsfile @department-of-veterans-affairs/backend-review-group Makefile @department-of-veterans-affairs/backend-review-group -Procfile @department-of-veterans-affairs/backend-review-group .devcontainer @department-of-veterans-affairs/backend-review-group @department-of-veterans-affairs/cto-engineers app/controllers/appeals_base_controller.rb @department-of-veterans-affairs/backend-review-group app/controllers/appeals_base_controller_v1.rb @department-of-veterans-affairs/backend-review-group @@ -644,13 +639,13 @@ app/sidekiq/vbms @department-of-veterans-affairs/benefits-dependents-management app/sidekiq/vre/create_ch31_submissions_report_job.rb @department-of-veterans-affairs/benefits-non-disability @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/vre/submit1900_job.rb @department-of-veterans-affairs/Benefits-Team-1 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/webhooks @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group +bin/fake_clamdscan @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group bin/git_blame @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group bin/rails @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group bin/rake @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group bin/rspec @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group bin/setup @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group bin/sidekiq_quiet @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group -clamav_tmp @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group config/application.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group config/betamocks @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group config/betamocks/services_config.yml @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group @@ -711,7 +706,7 @@ config/initializers/backtrace_silencers.rb @department-of-veterans-affairs/va-ap config/initializers/betamocks.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group config/initializers/bgs.rb @department-of-veterans-affairs/Benefits-Team-1 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group config/initializers/breakers.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group -config/initializers/clamav.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group +config/initializers/clamscan.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group config/initializers/config.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group config/initializers/cookie_rotation.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group config/initializers/covid_vaccine_facilities.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group @department-of-veterans-affairs/long-covid @@ -808,7 +803,6 @@ lib/caseflow @department-of-veterans-affairs/lighthouse-banana-peels @department lib/central_mail @department-of-veterans-affairs/lighthouse-banana-peels @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group lib/chip @department-of-veterans-affairs/vsa-healthcare-health-quest-1-backend @department-of-veterans-affairs/patient-check-in @department-of-veterans-affairs/backend-review-group lib/claim_letters @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group -lib/clamav @department-of-veterans-affairs/backend-review-group lib/common/client/base.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group lib/common/client/concerns/mhv_fhir_session_client.rb @department-of-veterans-affairs/vfs-mhv-medical-records @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group lib/common/client/concerns/mhv_jwt_session_client.rb @department-of-veterans-affairs/vfs-mhv-medical-records @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group @@ -820,7 +814,6 @@ lib/common/client/middleware/request/remove_cookies.rb @department-of-veterans-a lib/common/client/middleware/response/soap_parser.rb @department-of-veterans-affairs/backend-review-group lib/common/exceptions/open_id_service_error.rb @department-of-veterans-affairs/lighthouse-pivot lib/common/file_helpers.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group -lib/common/virus_scan.rb @department-of-veterans-affairs/backend-review-group lib/debt_management_center @department-of-veterans-affairs/vsa-debt-resolution @department-of-veterans-affairs/backend-review-group lib/decision_review @department-of-veterans-affairs/Benefits-Team-1 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group lib/decision_review_v1 @department-of-veterans-affairs/Benefits-Team-1 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group @@ -927,7 +920,6 @@ lib/search @department-of-veterans-affairs/va-api-engineers @department-of-veter lib/sentry @department-of-veterans-affairs/backend-review-group lib/sentry_logging.rb @department-of-veterans-affairs/backend-review-group lib/sftp_writer @department-of-veterans-affairs/backend-review-group @department-of-veterans-affairs/va-api-engineers -lib/shrine @department-of-veterans-affairs/backend-review-group @department-of-veterans-affairs/va-api-engineers lib/sidekiq/attr_package.rb @department-of-veterans-affairs/octo-identity @department-of-veterans-affairs/backend-review-group lib/sidekiq/error_tag.rb @department-of-veterans-affairs/backend-review-group @department-of-veterans-affairs/va-api-engineers lib/sidekiq/form526_backup_submission_process @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/backend-review-group @department-of-veterans-affairs/va-api-engineers @@ -1394,7 +1386,6 @@ spec/lib/sentry @department-of-veterans-affairs/va-api-engineers @department-of- spec/lib/sftp_writer @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/lib/sftp_writer/factory_spec.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/lib/sftp_writer/remote_spec.rb @department-of-veterans-affairs/backend-review-group -spec/lib/shrine @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/lib/sidekiq/attr_package_spec.rb @department-of-veterans-affairs/octo-identity @department-of-veterans-affairs/backend-review-group spec/lib/sidekiq/error_tag_spec.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/lib/sidekiq/form526_backup_submission_process @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/dbex-trex @department-of-veterans-affairs/benefits-disability-2 @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group diff --git a/.github/workflows/audit_service_tags.yml b/.github/workflows/audit_service_tags.yml index 9391b69f875..b81aad522b3 100644 --- a/.github/workflows/audit_service_tags.yml +++ b/.github/workflows/audit_service_tags.yml @@ -36,9 +36,10 @@ jobs: uses: docker/build-push-action@v5 with: build-args: | - BUNDLE_ENTERPRISE__CONTRIBSYS__COM=${{ env.BUNDLE_ENTERPRISE__CONTRIBSYS__COM }} - USER_ID=${{ env.VETS_API_USER_ID }} + sidekiq_license=${{ env.BUNDLE_ENTERPRISE__CONTRIBSYS__COM }} + userid=${{ env.VETS_API_USER_ID }} context: . + target: builder push: false load: true tags: vets-api @@ -47,8 +48,8 @@ jobs: - name: Setup Database run: | - docker-compose -f docker-compose.test.yml run web bash \ - -c "CI=true RAILS_ENV=test DISABLE_BOOTSNAP=true bundle exec parallel_test -n 13 -e 'bin/rails db:reset'" + docker-compose -f docker-compose.test.yml run vets-api bash \ + -c "CI=true RAILS_ENV=test DISABLE_BOOTSNAP=true parallel_test -n 13 -e 'bin/rails db:reset'" - name: Get changed files run: | @@ -59,6 +60,6 @@ jobs: - name: Run service tags audit controllers task run: | - docker-compose -f docker-compose.test.yml run -e CHANGED_FILES=${{ env.CHANGED_FILES }} web bash \ + docker-compose -f docker-compose.test.yml run -e CHANGED_FILES=${{ env.CHANGED_FILES }} vets-api bash \ -c "CI=true DISABLE_BOOTSNAP=true bundle exec rake service_tags:audit_controllers_ci" diff --git a/.github/workflows/code_checks.yml b/.github/workflows/code_checks.yml index fc904fdbbb8..1361e44f06d 100644 --- a/.github/workflows/code_checks.yml +++ b/.github/workflows/code_checks.yml @@ -54,9 +54,10 @@ jobs: uses: docker/build-push-action@v5 with: build-args: | - BUNDLE_ENTERPRISE__CONTRIBSYS__COM=${{ env.BUNDLE_ENTERPRISE__CONTRIBSYS__COM }} - USER_ID=${{ env.VETS_API_USER_ID }} + sidekiq_license=${{ env.BUNDLE_ENTERPRISE__CONTRIBSYS__COM }} + userid=${{ env.VETS_API_USER_ID }} context: . + target: builder push: false load: true tags: vets-api @@ -65,13 +66,13 @@ jobs: - name: Setup Database run: | - docker-compose -f docker-compose.test.yml run web bash \ - -c "CI=true RAILS_ENV=test DISABLE_BOOTSNAP=true bundle exec parallel_test -n 13 -e 'bin/rails db:reset'" + docker-compose -f docker-compose.test.yml run vets-api bash \ + -c "CI=true RAILS_ENV=test DISABLE_BOOTSNAP=true parallel_test -n 13 -e 'bin/rails db:reset'" - name: Run Specs timeout-minutes: 20 run: | - docker-compose -f docker-compose.test.yml run web bash \ + docker-compose -f docker-compose.test.yml run vets-api bash \ -c "CI=true DISABLE_BOOTSNAP=true bundle exec parallel_rspec spec/ modules/ -n 13 -o '--color --tty'" - name: Upload Coverage Report diff --git a/.gitignore b/.gitignore index 1faef9f5952..42aad621156 100644 --- a/.gitignore +++ b/.gitignore @@ -102,7 +102,3 @@ node_modules # Ignore public folder (used for local document uploads) public -# Ignore any files within clamav_tmp - -clamav_tmp/* -!/clamav_tmp/.keep diff --git a/Dockerfile b/Dockerfile index 2d403bc889e..80f2ad21a87 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,9 @@ -FROM ruby:3.2.4-slim-bookworm AS rubyimg +FROM ruby:3.2.4-slim-bookworm as rubyimg + +# XXX: using stretch here for pdftk dep, which is not availible after +# stretch (or in alpine) and is switched automatically to pdftk-java in buster +# https://github.com/department-of-veterans-affairs/va.gov-team/issues/3032 + FROM rubyimg AS modules WORKDIR /tmp @@ -8,64 +13,96 @@ COPY modules/ modules/ RUN find modules -type f ! \( -name Gemfile -o -name "*.gemspec" -o -path "*/lib/*/version.rb" \) -delete && \ find modules -type d -empty -delete -FROM rubyimg - -# Allow for setting ENV vars via --build-arg -ARG BUNDLE_ENTERPRISE__CONTRIBSYS__COM \ - RAILS_ENV=development \ - USER_ID=1000 -ENV RAILS_ENV=$RAILS_ENV \ - BUNDLE_ENTERPRISE__CONTRIBSYS__COM=$BUNDLE_ENTERPRISE__CONTRIBSYS__COM \ - BUNDLER_VERSION=2.4.9 - -RUN groupadd --gid $USER_ID nonroot \ - && useradd --uid $USER_ID --gid nonroot --shell /bin/bash --create-home nonroot --home-dir /app - -WORKDIR /app +### +# shared build/settings for all child images, reuse these layers yo +### +FROM rubyimg AS base +ARG userid=993 +SHELL ["/bin/bash", "-c"] +RUN groupadd -g $userid -r vets-api && \ + useradd -u $userid -r -m -d /srv/vets-api -g vets-api vets-api RUN apt-get update --fix-missing -RUN apt-get install -y poppler-utils build-essential libpq-dev git curl wget ca-certificates-java file \ - imagemagick pdftk \ - && apt-get clean \ - && rm -rf /var/cache/apt/archives/* /var/lib/apt/lists/* /tmp/* /var/tmp/* - +RUN DEBIAN_FRONTEND=noninteractive apt-get install -y ca-certificates-java && \ + DEBIAN_FRONTEND=noninteractive apt-get install -y dumb-init imagemagick pdftk poppler-utils curl \ + libpq5 vim libboost-all-dev clamav clamdscan clamav-daemon + +# The pki work below is for parity with the non-docker BRD deploys to mount certs into +# the container, we need to get rid of it and refactor the configuration bits into +# something more continer friendly in a later bunch of work +RUN mkdir -p /srv/vets-api/{clamav/database,pki/tls,secure,src} && \ + chown -R vets-api:vets-api /srv/vets-api && \ + ln -s /srv/vets-api/pki /etc/pki +# XXX: get rid of the CA trust manipulation when we have a better model for it +COPY config/ca-trust/* /usr/local/share/ca-certificates/ +# rename .pem files to .crt because update-ca-certificates ignores files that are not .crt +RUN cd /usr/local/share/ca-certificates ; for i in *.pem ; do mv $i ${i/pem/crt} ; done ; update-ca-certificates # Relax ImageMagick PDF security. See https://stackoverflow.com/a/59193253. RUN sed -i '/rights="none" pattern="PDF"/d' /etc/ImageMagick-6/policy.xml - - -# Install fwdproxy.crt into trust store -# Relies on update-ca-certificates being run in following step -COPY config/ca-trust/*.crt /usr/local/share/ca-certificates/ - -# Download VA Certs -COPY ./import-va-certs.sh . -RUN ./import-va-certs.sh - -COPY config/clamd.conf /etc/clamav/clamd.conf - -RUN mkdir -p /clamav_tmp && \ - chown -R nonroot:nonroot /clamav_tmp && \ - chmod 777 /clamav_tmp - - -ENV LANG=C.UTF-8 \ - BUNDLE_JOBS=4 \ - BUNDLE_PATH=/usr/local/bundle/cache \ - BUNDLE_RETRY=3 - +WORKDIR /srv/vets-api/src + +### +# dev stage; use --target=development to stop here +# Be sure to pass required ARGs as `--build-arg` +# This stage useful for mounting your local checkout with compose +# into the container to dev against. +### +FROM base AS development + +ARG sidekiq_license +ARG rails_env=development + +ENV BUNDLE_ENTERPRISE__CONTRIBSYS__COM=$sidekiq_license +ENV RAILS_ENV=$rails_env +ENV BUNDLER_VERSION=2.4.9 + +# only extra dev/build opts go here, common packages go in base 👆 +RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y \ + git build-essential libxml2-dev libxslt-dev libpq-dev +COPY --chown=vets-api:vets-api config/freshclam.conf docker-entrypoint.sh ./ +USER vets-api +# XXX: this is tacky +RUN freshclam --config-file freshclam.conf +RUN gem install vtk +ENTRYPOINT ["/usr/bin/dumb-init", "--", "./docker-entrypoint.sh"] RUN gem install bundler:${BUNDLER_VERSION} --no-document -COPY --from=modules /tmp/modules modules/ -COPY Gemfile Gemfile.lock ./ -RUN bundle install \ +### +# build stage; use --target=builder to stop here +# Also be sure to add build-args from development stage above +# +# This is development with the app copied in and built. The build results are used in +# prod below, but also useful if you want to have a container with the app and not +# mount your local checkout. +### +FROM development AS builder +# XXX: move modules/ to seperate repos so we can only copy Gemfile* and install a slim layer +ARG bundler_opts + +COPY --chown=vets-api:vets-api Gemfile Gemfile.lock ./ +COPY --chown=vets-api:vets-api --from=modules /tmp/modules modules/ + +RUN bundle install --binstubs="${BUNDLE_APP_CONFIG}/bin" $bundler_opts \ && rm -rf /usr/local/bundle/cache/*.gem \ && find /usr/local/bundle/gems/ -name "*.c" -delete \ && find /usr/local/bundle/gems/ -name "*.o" -delete \ && find /usr/local/bundle/gems/ -name ".git" -type d -prune -execdir rm -rf {} + -COPY --chown=nonroot:nonroot . . - -EXPOSE 3000 - -USER nonroot -CMD ["bundle", "exec", "rails", "server", "-b", "0.0.0.0"] +COPY --chown=vets-api:vets-api . . +USER vets-api + +### +# prod stage; default if no target given +# to build prod you probably want options like below to get a good build +# --build-arg sidekiq_license="$BUNDLE_ENTERPRISE__CONTRIBSYS__COM" --build-arg rails_env=production --build-arg bundler_opts="--no-cache --without development test" +# This inherits from base again to avoid bringing in extra built time binary packages +### +FROM base AS production + +ENV RAILS_ENV=production +COPY --from=builder $BUNDLE_APP_CONFIG $BUNDLE_APP_CONFIG +COPY --from=builder --chown=vets-api:vets-api /srv/vets-api/src ./ +COPY --from=builder --chown=vets-api:vets-api /srv/vets-api/clamav/database ../clamav/database +RUN if [ -d certs-tmp ] ; then cd certs-tmp ; for i in * ; do cp $i /usr/local/share/ca-certificates/${i/pem/crt} ; done ; fi && update-ca-certificates +USER vets-api +ENTRYPOINT ["/usr/bin/dumb-init", "--", "./docker-entrypoint.sh"] diff --git a/Gemfile b/Gemfile index ce4bd571ea4..f3a4bf79ce5 100644 --- a/Gemfile +++ b/Gemfile @@ -55,7 +55,7 @@ gem 'bootsnap', require: false gem 'breakers' gem 'carrierwave' gem 'carrierwave-aws' -gem 'clamav-client', require: 'clamav/client' +gem 'clam_scan' gem 'combine_pdf' gem 'config' gem 'connect_vbms', git: 'https://github.com/adhocteam/connect_vbms', tag: 'v2.0.0.rc', require: 'vbms' diff --git a/Gemfile.lock b/Gemfile.lock index e3cf8f240a5..465033cee09 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -301,7 +301,7 @@ GEM cork nap open4 (~> 1.3) - clamav-client (3.2.0) + clam_scan (0.0.2) cliver (0.3.2) coderay (1.1.3) coercible (1.0.0) @@ -596,12 +596,9 @@ GEM kramdown (~> 2.0) language_server-protocol (3.17.0.3) libdatadog (5.0.0.1.0) - libdatadog (5.0.0.1.0-aarch64-linux) libdatadog (5.0.0.1.0-x86_64-linux) libddwaf (1.14.0.0.0) ffi (~> 1.0) - libddwaf (1.14.0.0.0-aarch64-linux) - ffi (~> 1.0) libddwaf (1.14.0.0.0-java) ffi (~> 1.0) libddwaf (1.14.0.0.0-x86_64-linux) @@ -1079,7 +1076,6 @@ GEM zeitwerk (2.6.13) PLATFORMS - aarch64-linux java ruby x64-mingw32 @@ -1115,7 +1111,7 @@ DEPENDENCIES carrierwave-aws check_in! claims_api! - clamav-client + clam_scan combine_pdf config connect_vbms! diff --git a/Makefile b/Makefile index cac52538ac9..16c7d952307 100644 --- a/Makefile +++ b/Makefile @@ -7,9 +7,16 @@ else ENV_ARG := dev endif +ifdef clam + FOREMAN_ARG := all=1 +else + FOREMAN_ARG := all=1,clamd=0,freshclam=0 +endif + + COMPOSE_DEV := docker-compose COMPOSE_TEST := docker-compose -f docker-compose.test.yml -BASH := run --rm --service-ports web bash +BASH := run --rm --service-ports vets-api bash BASH_DEV := $(COMPOSE_DEV) $(BASH) -c BASH_TEST := $(COMPOSE_TEST) $(BASH) --login -c SPEC_PATH := spec/ modules/ @@ -110,9 +117,9 @@ spec: ## Runs spec tests .PHONY: spec_parallel_setup spec_parallel_setup: ## Setup the parallel test dbs. This resets the current test db, as well as the parallel test dbs ifeq ($(ENV_ARG), dev) - @$(BASH_DEV) "RAILS_ENV=test DISABLE_BOOTSNAP=true bundle exec parallel_test -e 'bundle exec rake db:reset db:migrate'" + @$(BASH_DEV) "RAILS_ENV=test DISABLE_BOOTSNAP=true parallel_test -e 'bundle exec rake db:reset'" else - @$(COMPOSE_TEST) $(BASH) -c "RAILS_ENV=test DISABLE_BOOTSNAP=true parallel_test -e 'bundle exec rake db:reset db:migrate'" + @$(COMPOSE_TEST) $(BASH) -c "RAILS_ENV=test DISABLE_BOOTSNAP=true parallel_test -e 'bundle exec rake db:reset'" endif .PHONY: spec_parallel @@ -124,14 +131,14 @@ else endif .PHONY: up -up: db ## Starts the server and associated services with docker-compose - @$(BASH_DEV) "rm -f tmp/pids/server.pid && foreman start -m all=1" +up: db ## Starts the server and associated services with docker-compose, use `clam=1 make up` to run ClamAV + @$(BASH_DEV) "rm -f tmp/pids/server.pid && foreman start -m ${FOREMAN_ARG}" # NATIVE COMMANDS .PHONY: native-up native-up: bundle install - foreman start -m all=1 + foreman start -m ${FOREMAN_ARG} .PHONY: native-lint native-lint: diff --git a/Procfile b/Procfile index 9af216e5db4..074c2758193 100644 --- a/Procfile +++ b/Procfile @@ -1,2 +1,4 @@ web: bundle exec puma -p 3000 -C ./config/puma.rb job: bundle exec sidekiq -q critical,4 -q tasker,3 -q default,2 -q low,1 +freshclam: /usr/bin/freshclam -d --config-file=config/freshclam.conf +clamd: /usr/sbin/clamd -c config/clamd.conf diff --git a/app/uploaders/uploader_virus_scan.rb b/app/uploaders/uploader_virus_scan.rb index 846adc6d175..4bd293a83d7 100644 --- a/app/uploaders/uploader_virus_scan.rb +++ b/app/uploaders/uploader_virus_scan.rb @@ -16,14 +16,13 @@ class VirusFoundError < StandardError def validate_virus_free(file) return unless Rails.env.production? - temp_file_path = Common::FileHelpers.generate_clamav_temp_file(file.read) + temp_file_path = Common::FileHelpers.generate_temp_file(file.read) result = Common::VirusScan.scan(temp_file_path) File.delete(temp_file_path) - # Common::VirusScan result will return true or false - unless result # unless safe + unless result.safe? file.delete - raise VirusFoundError, "Virus Found + #{temp_file_path}" + raise VirusFoundError, result.body end end end diff --git a/bin/fake_clamdscan b/bin/fake_clamdscan new file mode 100755 index 00000000000..63a31f0b72c --- /dev/null +++ b/bin/fake_clamdscan @@ -0,0 +1,5 @@ +#!/bin/bash +# frozen_string_literal: true + +echo 'Fake scanning file with fake_clamdscan script' +exit 0 diff --git a/clamav_tmp/.keep b/clamav_tmp/.keep deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/config/clamd.conf b/config/clamd.conf index 1c12245cf51..52595bda7b5 100644 --- a/config/clamd.conf +++ b/config/clamd.conf @@ -1,7 +1,5 @@ Foreground yes +DatabaseDirectory /srv/vets-api/clamav/database +LocalSocket /srv/vets-api/clamav/clamd.ctl TCPSocket 3310 -TCPAddr 127.0.0.1 - -LogSyslog yes -LogVerbose yes -ExtendedDetectionInfo yes +TCPAddr 127.0.0.1 \ No newline at end of file diff --git a/config/freshclam.conf b/config/freshclam.conf index f43853975e0..fa18e68d88b 100644 --- a/config/freshclam.conf +++ b/config/freshclam.conf @@ -3,5 +3,5 @@ PidFile /srv/vets-api/clamav/freshclam.pid Checks 8 DatabaseDirectory /srv/vets-api/clamav/database PrivateMirror dsva-vetsgov-utility-clamav.s3-us-gov-west-1.amazonaws.com -NotifyClamd /app/config/clamd.conf +NotifyClamd /srv/vets-api/src/config/clamd.conf ReceiveTimeout 600 diff --git a/config/initializers/clamav.rb b/config/initializers/clamav.rb deleted file mode 100644 index bc13ae935c3..00000000000 --- a/config/initializers/clamav.rb +++ /dev/null @@ -1,6 +0,0 @@ -# frozen_string_literal: true - -if Rails.env.development? - ENV['CLAMD_TCP_HOST'] = Settings.clamav.host - ENV['CLAMD_TCP_PORT'] = Settings.clamav.port -end diff --git a/config/initializers/clamscan.rb b/config/initializers/clamscan.rb new file mode 100644 index 00000000000..2f864ddfb9a --- /dev/null +++ b/config/initializers/clamscan.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +ClamScan.configure do |clam_config| + clam_config.client_location = Settings.binaries.clamdscan +end diff --git a/config/settings.local.yml.example b/config/settings.local.yml.example index c9db66102e6..ad55d272e4a 100644 --- a/config/settings.local.yml.example +++ b/config/settings.local.yml.example @@ -6,11 +6,11 @@ # The relative path to department-of-veterans-affairs/vets-api-mockdata # cache_dir: ../vets-api-mockdata -# clamav: +# binaries: + # For NATIVE and DOCKER installation # A "virus scanner" that always returns success for development purposes - # mock: true - # host: '0.0.0.0' - # port: '33100' + # NOTE: You may need to specify a full path instead of a relative path + # clamdscan: ./bin/fake_clamdscan # NOTE: This file is excluded by railsconfig in the test env. # Use config/settings/test.local.yml instead. diff --git a/config/settings.yml b/config/settings.yml index e3f5af2d20c..c1cd8bdb74f 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -97,13 +97,6 @@ binaries: pdftk: pdftk clamdscan: /usr/bin/clamdscan -clamav: - mock: false - # host & port here are only used in development here: - # config/initializers/clamav.rb - host: 'clamav' - port: '3310' - db_encryption_key: f01ff8ebd1a2b053ad697ae1f0d86adb database_url: postgis:///vets-api diff --git a/docker-compose-clamav.yml b/docker-compose-clamav.yml deleted file mode 100644 index 239aa2936a3..00000000000 --- a/docker-compose-clamav.yml +++ /dev/null @@ -1,10 +0,0 @@ -version: '3.4' -services: - clamav: - volumes: - - shared-vol:/vets-api - image: clamav/clamav - ports: - - 33100:3310 -volumes: - shared-vol: diff --git a/docker-compose-deps.yml b/docker-compose-deps.yml index 489576324b8..b1ab99a4d31 100644 --- a/docker-compose-deps.yml +++ b/docker-compose-deps.yml @@ -13,11 +13,4 @@ services: - ./data:/var/lib/postgresql/data:cached ports: - "54320:5432" - clamav: - volumes: - - shared-vol:/vets-api - image: clamav/clamav - ports: - - 33100:3310 -volumes: - shared-vol: + diff --git a/docker-compose.review.yml b/docker-compose.review.yml index 4ce4b1999f9..43769083ab7 100644 --- a/docker-compose.review.yml +++ b/docker-compose.review.yml @@ -1,70 +1,55 @@ version: '3.4' - -x-app: &common - build: - args: - BUNDLE_ENTERPRISE__CONTRIBSYS__COM: "${BUNDLE_ENTERPRISE__CONTRIBSYS__COM}" - USER_ID: ${VETS_API_USER_ID} - context: . - environment: - RAILS_ENV: development - BUNDLE_ENTERPRISE__CONTRIBSYS__COM: "${BUNDLE_ENTERPRISE__CONTRIBSYS__COM}" - "Settings.database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_development}?pool=4" - "Settings.test_database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_test}" - "Settings.redis.app_data.url": "redis://redis:6379" - "Settings.redis.sidekiq.url": "redis://redis:6379" - "Settings.redis.rails_cache.url": "redis://redis:6379" - "Settings.saml_ssoe.idp_metadata_file": "config/ssoe_idp_sqa_metadata_isam.xml" - "Settings.betamocks.cache_dir": "config/vets-api-mockdata" - image: vets-api:${DOCKER_IMAGE:-latest} - restart: unless-stopped - volumes: - - "../vets-api-mockdata:/cache" - - ../.secret:/srv/vets-api/secret:cached - - ../.pki:/srv/vets-api/pki:cached - - shared-vol:/tmp - working_dir: /app - depends_on: - - clamav - - postgres - - redis - links: - - clamav - - postgres - - redis - services: - clamav: - image: clamav/clamav - restart: unless-stopped - ports: - - 3310:3310 - volumes: - - shared-vol:/vets-api redis: image: redis:6.2-alpine restart: unless-stopped - ports: - - 6379:6379 postgres: - command: postgres -c shared_preload_libraries=pg_stat_statements -c pg_stat_statements.track=all -c max_connections=200 + image: mdillon/postgis:11-alpine environment: POSTGRES_PASSWORD: "${POSTGRES_PASSWORD:-password}" POSTGRES_USER: "${POSTGRES_USER:-postgres}" - PGDATA: /tmp - image: postgis/postgis:14-3.3-alpine + volumes: + - db-data:/var/lib/postgresql/data:cached ports: - - 5432:5432 + - "54320:5432" + restart: unless-stopped + vets-api: + build: + context: . + target: development + args: + sidekiq_license: "${BUNDLE_ENTERPRISE__CONTRIBSYS__COM}" + userid: "${VETS_API_USER_ID}" + command: > + bash -c "bundle exec rake db:migrate || bundle exec rake db:setup db:migrate + && touch tmp/caching-dev.txt && foreman start -m all=1,clamd=0,freshclam=0" + image: "vets-api:${DOCKER_IMAGE:-latest}" volumes: - - ./data:/var/lib/postgresql/data:cached - web: - <<: *common - command: bash -c "bundle exec rake db:migrate || bundle exec rake db:reset db:migrate && bundle exec rails s -b 0.0.0.0" + - .:/srv/vets-api/src:cached + - dev_bundle:/usr/local/bundle + - ../.secret:/srv/vets-api/secret:cached + - ../.pki:/srv/vets-api/pki:cached ports: - - 3000:3000 - worker: - <<: *common - command: bundle exec sidekiq -q critical,4 -q tasker,3 -q default,2 -q low,1 - + - "3000:3000" + environment: + "Settings.database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_development}?pool=4" + "Settings.test_database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_test}?pool=4" + "Settings.redis.app_data.url": "redis://redis:6379" + "Settings.redis.sidekiq.url": "redis://redis:6379" + "Settings.redis.rails_cache.url": "redis://redis:6379" + "Settings.binaries.clamdscan": "clamscan" # Not running a separate process within the container for clamdscan, so we use clamscan which requires no daemon + POSTGRES_HOST: "${POSTGRES_HOST:-postgres}" + POSTGRES_PORT: "${POSTGRES_PORT:-5432}" + POSTGRES_USER: "${POSTGRES_USER:-postgres}" + POSTGRES_PASSWORD: "${POSTGRES_PASSWORD:-password}" + PUMA_THREADS: "${PUMA_THREADS:-4}" + depends_on: + - postgres + - redis + links: + - postgres + - redis + restart: unless-stopped volumes: - shared-vol: + db-data: + dev_bundle: diff --git a/docker-compose.test.yml b/docker-compose.test.yml index f532a0a2211..907cf5c2d27 100644 --- a/docker-compose.test.yml +++ b/docker-compose.test.yml @@ -8,7 +8,7 @@ services: environment: POSTGRES_PASSWORD: "${POSTGRES_PASSWORD:-password}" POSTGRES_USER: "${POSTGRES_USER:-postgres}" - web: + vets-api: build: context: . target: development @@ -22,6 +22,7 @@ services: environment: "Settings.database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_development}?pool=4" "Settings.test_database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_test}" + "Settings.binaries.clamdscan": "clamscan" # Not running a separate process within the container for clamdscan, so we use clamscan which requires no daemon "Settings.redis.app_data.url": "redis://redis:6379" "Settings.redis.sidekiq.url": "redis://redis:6379" POSTGRES_HOST: "${POSTGRES_HOST:-postgres}" diff --git a/docker-compose.yml b/docker-compose.yml index dcac6012677..bf46aa633b9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,61 +1,49 @@ -version: '3.8' - -x-app: &common - build: - args: - BUNDLE_ENTERPRISE__CONTRIBSYS__COM: "${BUNDLE_ENTERPRISE__CONTRIBSYS__COM}" - context: . - environment: - BUNDLE_ENTERPRISE__CONTRIBSYS__COM: "${BUNDLE_ENTERPRISE__CONTRIBSYS__COM}" - "Settings.database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_development}?pool=4" - "Settings.test_database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_test}" - "Settings.redis.app_data.url": "redis://redis:6379" - "Settings.redis.sidekiq.url": "redis://redis:6379" - "Settings.redis.rails_cache.url": "redis://redis:6379" - image: vets-api:${DOCKER_IMAGE:-latest} - volumes: - - "../vets-api-mockdata:/cache" - - .:/app:cached - - shared-vol:/tmp - working_dir: /app - depends_on: - - clamav - - postgres - - redis - links: - - clamav - - postgres - - redis - +version: '3.4' services: - clamav: - image: clamav/clamav - ports: - - 33100:3310 - volumes: - - shared-vol:/vets-api redis: image: redis:6.2-alpine ports: - - 63790:6379 + - "63790:6379" postgres: - command: postgres -c shared_preload_libraries=pg_stat_statements -c pg_stat_statements.track=all -c max_connections=200 + image: mdillon/postgis:11-alpine environment: POSTGRES_PASSWORD: "${POSTGRES_PASSWORD:-password}" POSTGRES_USER: "${POSTGRES_USER:-postgres}" - PGDATA: /tmp - image: postgis/postgis:14-3.3-alpine - ports: - - 54320:5432 volumes: - ./data:/var/lib/postgresql/data:cached - web: - <<: *common ports: - - 3000:3000 - worker: - <<: *common - command: bundle exec sidekiq -q critical,4 -q tasker,3 -q default,2 -q low,1 + - "54320:5432" + vets-api: + build: + context: . + target: development + args: + sidekiq_license: "${BUNDLE_ENTERPRISE__CONTRIBSYS__COM}" + userid: "${VETS_API_USER_ID}" + image: "vets-api:${DOCKER_IMAGE:-latest}" + volumes: + - .:/srv/vets-api/src:cached + - "../vets-api-mockdata:/cache" + - dev_bundle:/usr/local/bundle + ports: + - "3000:3000" + environment: + "Settings.database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_development}?pool=4" + "Settings.test_database_url": "postgis://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-password}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/${POSTGRES_DATABASE:-vets_api_test}" + "Settings.redis.app_data.url": "redis://redis:6379" + "Settings.redis.sidekiq.url": "redis://redis:6379" + "Settings.binaries.clamdscan": "clamscan" # Not running a separate process within the container for clamdscan, so we use clamscan which requires no daemon + POSTGRES_HOST: "${POSTGRES_HOST:-postgres}" + POSTGRES_PORT: "${POSTGRES_PORT:-5432}" + POSTGRES_USER: "${POSTGRES_USER:-postgres}" + POSTGRES_PASSWORD: "${POSTGRES_PASSWORD:-password}" + depends_on: + - postgres + - redis + links: + - postgres + - redis volumes: - shared-vol: + db-data: + dev_bundle: diff --git a/lib/clamav/commands/patch_scan_command.rb b/lib/clamav/commands/patch_scan_command.rb deleted file mode 100644 index 0a3586b9ff0..00000000000 --- a/lib/clamav/commands/patch_scan_command.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -## Monkey Patch for the clamav SCAN COMMAND -## file path needs to be vets-api/ because of shared volumes - -# clamav-client - ClamAV client -# Copyright (C) 2014 Franck Verrot - -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. - -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. - -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -require 'clamav/commands/command' - -module ClamAV - module Commands - class PatchScanCommand < Command - def initialize(path, path_finder = Util) - @path = path - @path_finder = path_finder - super() - end - - def call(conn) - @path_finder.path_to_files(@path).map { |file| scan_file(conn, file) } - end - - def scan_file(conn, file) - stripped_filename = file.gsub(%r{^clamav_tmp/}, '') # need to send the file - get_status_from_response(conn.send_request("SCAN /vets-api/#{stripped_filename}")) - end - end - end -end diff --git a/lib/clamav/patch_client.rb b/lib/clamav/patch_client.rb deleted file mode 100644 index ac36546e91c..00000000000 --- a/lib/clamav/patch_client.rb +++ /dev/null @@ -1,76 +0,0 @@ -# frozen_string_literal: true - -# clamav-client - ClamAV client -# Copyright (C) 2014 Franck Verrot - -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. - -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. - -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -require 'clamav/connection' -require 'clamav/commands/ping_command' -require 'clamav/commands/quit_command' -require 'clamav/commands/scan_command' -require 'clamav/commands/instream_command' -require 'clamav/util' -require 'clamav/wrappers/new_line_wrapper' -require 'clamav/wrappers/null_termination_wrapper' -require_relative 'commands/patch_scan_command' - -module ClamAV - class PatchClient - def initialize(connection = default_connection) - @connection = connection - connection.establish_connection - end - - def execute(command) - command.call(@connection) - end - - def default_connection - ClamAV::Connection.new( - socket: resolve_default_socket, - wrapper: ::ClamAV::Wrappers::NewLineWrapper.new - ) - end - - def resolve_default_socket - unix_socket, tcp_host, tcp_port = ENV.values_at('CLAMD_UNIX_SOCKET', 'CLAMD_TCP_HOST', 'CLAMD_TCP_PORT') - if tcp_host && tcp_port - ::TCPSocket.new(tcp_host, tcp_port) - else - ::UNIXSocket.new(unix_socket || '/var/run/clamav/clamd.ctl') - end - end - - def ping - execute Commands::PingCommand.new - end - - def safe?(target) - return instream(target).virus_name.nil? if target.is_a?(StringIO) - - scan(target).all? { |file| file.virus_name.nil? } - end - - private - - def instream(io) - execute Commands::InstreamCommand.new(io) - end - - def scan(file_path) - execute Commands::PatchScanCommand.new(file_path) - end - end -end diff --git a/lib/common/file_helpers.rb b/lib/common/file_helpers.rb index 3fb1ebe5e75..fb49eda26cc 100644 --- a/lib/common/file_helpers.rb +++ b/lib/common/file_helpers.rb @@ -22,22 +22,5 @@ def generate_temp_file(file_body, file_name = nil) file_path end - - def generate_clamav_temp_file(file_body, file_name = nil) - file_name = SecureRandom.hex if file_name.nil? - clamav_directory = Rails.root.join('clamav_tmp') - unless Dir.exist?(clamav_directory) - # Create the directory if it doesn't exist - Dir.mkdir(clamav_directory) - end - - file_path = "clamav_tmp/#{file_name}" - - File.open(file_path, 'wb') do |file| - file.write(file_body) - end - - file_path - end end end diff --git a/lib/common/virus_scan.rb b/lib/common/virus_scan.rb index aefde7e31b5..e221b911faa 100644 --- a/lib/common/virus_scan.rb +++ b/lib/common/virus_scan.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -require 'clamav/commands/patch_scan_command' -require 'clamav/patch_client' - module Common module VirusScan module_function @@ -11,15 +8,10 @@ def scan(file_path) # `clamd` runs within service group, needs group read File.chmod(0o640, file_path) - if mock_enabled? - true - else - ClamAV::PatchClient.new.safe?(file_path) # patch to call our class - end - end - - def mock_enabled? - Settings.clamav.mock + # NOTE: If using custom_args, no other arguments can be passed to + # ClamScan::Client.scan. All other arguments will be ignored + args = ['-c', Rails.root.join('config', 'clamd.conf').to_s, file_path] + ClamScan::Client.scan(custom_args: args) end end end diff --git a/lib/shrine/plugins/validate_virus_free.rb b/lib/shrine/plugins/validate_virus_free.rb index 9a4cf6aca19..260ed57d1c0 100644 --- a/lib/shrine/plugins/validate_virus_free.rb +++ b/lib/shrine/plugins/validate_virus_free.rb @@ -9,11 +9,9 @@ module ValidateVirusFree module AttacherMethods def validate_virus_free(message: nil) Datadog::Tracing.trace('Scan Upload for Viruses') do - file_to_scan = get.download - temp_file_path = Common::FileHelpers.generate_clamav_temp_file(file_to_scan) - result = Common::VirusScan.scan(temp_file_path) - File.delete(temp_file_path) - result || add_error_msg(message || "Virus Found + #{temp_file_path}") + cached_path = get.download.path + result = Common::VirusScan.scan(cached_path) + result.safe? || add_error_msg(message || result.body) end end diff --git a/spec/lib/shrine/plugins/validate_virus_free_spec.rb b/spec/lib/shrine/plugins/validate_virus_free_spec.rb index 2bb879feec6..b98f9d56503 100644 --- a/spec/lib/shrine/plugins/validate_virus_free_spec.rb +++ b/spec/lib/shrine/plugins/validate_virus_free_spec.rb @@ -29,23 +29,31 @@ def errors context 'with errors' do before do - allow(Common::VirusScan).to receive(:scan).and_return(false) + allow(ClamScan.configuration).to receive(:client_location).and_return('found') end context 'while in development' do it 'logs an error message if clamd is not running' do expect(Rails.env).to receive(:development?).and_return(true) expect(Rails.logger).to receive(:error).with(/PLEASE START CLAMD/) - result = instance.validate_virus_free(message: 'nodename nor servname provided') + allow(ClamScan::Client).to receive(:scan) + .and_return(instance_double('ClamScan::Response', + safe?: false, + body: 'ERROR: Could not lookup : nodename nor servname provided, or not known')) + + result = instance.validate_virus_free expect(result).to be(true) end end context 'with the default error message' do it 'adds an error if clam scan returns not safe' do + allow(ClamScan::Client).to receive(:scan) + .and_return(instance_double('ClamScan::Response', safe?: false, body: nil)) + result = instance.validate_virus_free expect(result).to be(false) - expect(instance.errors).to include(match(/Virus Found/)) + expect(instance.errors).to eq(['virus or malware detected']) end end @@ -53,6 +61,9 @@ def errors let(:message) { 'oh noes!' } it 'adds an error with a custom error message if clam scan returns not safe' do + allow(ClamScan::Client).to receive(:scan) + .and_return(instance_double('ClamScan::Response', safe?: false)) + result = instance.validate_virus_free(message:) expect(result).to be(false) expect(instance.errors).to eq(['oh noes!']) @@ -60,18 +71,21 @@ def errors end end - context 'it returns safe' do - before do - allow(Common::VirusScan).to receive(:scan).and_return(true) - end + it 'does not add an error if clam scan returns safe' do + allow(ClamScan::Client).to receive(:scan) + .and_return(instance_double('ClamScan::Response', safe?: true)) + + expect(instance).not_to receive(:add_error_msg) + result = instance.validate_virus_free + expect(result).to be(true) + end - it 'does not add an error if clam scan returns safe' do - allow_any_instance_of(ClamAV::PatchClient).to receive(:safe?).and_return(true) + it 'changes group permissions of the uploaded file' do + allow(ClamScan::Client).to receive(:scan) + .and_return(instance_double('ClamScan::Response', safe?: true)) - expect(instance).not_to receive(:add_error_msg) - result = instance.validate_virus_free - expect(result).to be(true) - end + expect(File).to receive(:chmod).with(0o640, 'foo/bar.jpg').and_return(1) + instance.validate_virus_free end end end diff --git a/spec/models/persistent_attachments/dependency_claim_spec.rb b/spec/models/persistent_attachments/dependency_claim_spec.rb index 7e9ac88134a..a4339736e80 100644 --- a/spec/models/persistent_attachments/dependency_claim_spec.rb +++ b/spec/models/persistent_attachments/dependency_claim_spec.rb @@ -6,16 +6,13 @@ let(:file) { Rails.root.join('spec', 'fixtures', 'files', 'marriage-certificate.pdf') } let(:instance) { described_class.new(form_id: '686C-674') } - before do - allow(Common::VirusScan).to receive(:scan).and_return(true) - end - it 'sets a guid on initialize' do expect(instance.guid).to be_a(String) end it 'allows adding a file' do - allow_any_instance_of(ClamAV::PatchClient).to receive(:safe?).and_return(true) + allow(ClamScan::Client).to receive(:scan) + .and_return(instance_double('ClamScan::Response', safe?: true)) instance.file = file.open expect(instance.valid?).to be(true) expect(instance.file.shrine_class).to be(ClaimDocumentation::Uploader) diff --git a/spec/models/persistent_attachments/lgy_claim_spec.rb b/spec/models/persistent_attachments/lgy_claim_spec.rb index 230b9db1467..b0c06a6d131 100644 --- a/spec/models/persistent_attachments/lgy_claim_spec.rb +++ b/spec/models/persistent_attachments/lgy_claim_spec.rb @@ -6,16 +6,13 @@ let(:file) { Rails.root.join('spec', 'fixtures', 'files', 'marriage-certificate.pdf') } let(:instance) { described_class.new(form_id: '28-1880') } - before do - allow(Common::VirusScan).to receive(:scan).and_return(true) - end - it 'sets a guid on initialize' do expect(instance.guid).to be_a(String) end it 'allows adding a file' do - allow_any_instance_of(ClamAV::PatchClient).to receive(:safe?).and_return(true) + allow(ClamScan::Client).to receive(:scan) + .and_return(instance_double('ClamScan::Response', safe?: true)) instance.file = file.open expect(instance.valid?).to be(true) expect(instance.file.shrine_class).to be(ClaimDocumentation::Uploader) diff --git a/spec/models/persistent_attachments/pension_burial_spec.rb b/spec/models/persistent_attachments/pension_burial_spec.rb index 2dd2bdb0ec4..86485ad393d 100644 --- a/spec/models/persistent_attachments/pension_burial_spec.rb +++ b/spec/models/persistent_attachments/pension_burial_spec.rb @@ -6,16 +6,13 @@ let(:file) { Rails.root.join('spec', 'fixtures', 'files', 'doctors-note.pdf') } let(:instance) { described_class.new(form_id: 'T-123') } - before do - allow(Common::VirusScan).to receive(:scan).and_return(true) - end - it 'sets a guid on initialize' do expect(instance.guid).to be_a(String) end it 'allows adding a file' do - allow_any_instance_of(ClamAV::PatchClient).to receive(:safe?).and_return(true) + allow(ClamScan::Client).to receive(:scan) + .and_return(instance_double('ClamScan::Response', safe?: true)) instance.file = file.open expect(instance.valid?).to be(true) expect(instance.file.shrine_class).to be(ClaimDocumentation::Uploader) diff --git a/spec/requests/claim_documents_spec.rb b/spec/requests/claim_documents_spec.rb index a02810c323b..f927db74dfa 100644 --- a/spec/requests/claim_documents_spec.rb +++ b/spec/requests/claim_documents_spec.rb @@ -6,8 +6,8 @@ before do allow(Rails.logger).to receive(:info) allow(Rails.logger).to receive(:error) - allow(Common::VirusScan).to receive(:scan).and_return(true) - allow_any_instance_of(Common::VirusScan).to receive(:scan).and_return(true) + allow(ClamScan::Client).to receive(:scan) + .and_return(instance_double('ClamScan::Response', safe?: true)) end context 'with a valid file' do diff --git a/spec/simplecov_helper.rb b/spec/simplecov_helper.rb index 78b15d8fbb7..cb82fd49798 100644 --- a/spec/simplecov_helper.rb +++ b/spec/simplecov_helper.rb @@ -39,6 +39,7 @@ def merge_results def self.add_filters add_filter 'app/controllers/concerns/accountable.rb' + add_filter 'config/initializers/clamscan.rb' add_filter 'lib/apps/configuration.rb' add_filter 'lib/apps/responses/response.rb' add_filter 'lib/config_helper.rb' diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ac0ff59beac..593140521e4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -26,6 +26,7 @@ add_filter 'app/controllers/concerns/accountable.rb' add_filter 'app/models/in_progress_disability_compensation_form.rb' add_filter 'app/serializers/appeal_serializer.rb' + add_filter 'config/initializers/clamscan.rb' add_filter 'lib/apps/configuration.rb' add_filter 'lib/apps/responses/response.rb' add_filter 'lib/config_helper.rb' diff --git a/spec/support/uploader_helpers.rb b/spec/support/uploader_helpers.rb index f7559ce71fb..bb2cf7abd36 100644 --- a/spec/support/uploader_helpers.rb +++ b/spec/support/uploader_helpers.rb @@ -7,8 +7,14 @@ module UploaderHelpers module ClassMethods def stub_virus_scan + let(:result) do + { + safe?: true + } + end + before do - allow(Common::VirusScan).to receive(:scan).and_return(true) + allow(Common::VirusScan).to receive(:scan).and_return(OpenStruct.new(result)) end end end diff --git a/spec/uploaders/uploader_virus_scan_spec.rb b/spec/uploaders/uploader_virus_scan_spec.rb index 0726bde9961..6f8d0a330f8 100644 --- a/spec/uploaders/uploader_virus_scan_spec.rb +++ b/spec/uploaders/uploader_virus_scan_spec.rb @@ -24,15 +24,19 @@ def store_image end context 'with a virus' do - let(:result) { false } + let(:result) do + { + safe?: false, + body: 'virus found' + } + end it 'raises an error' do - allow(Common::VirusScan).to receive(:scan).and_return(false) expect(Rails.env).to receive(:production?).and_return(true) expect(file).to receive(:delete) expect { store_image }.to raise_error( - UploaderVirusScan::VirusFoundError + UploaderVirusScan::VirusFoundError, 'virus found' ) end end