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.

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

Chore: Enable gosec #140

Merged
merged 5 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/build-api.yml
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loboda4450 we still let something slip through 😅

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh, that's embarrassing....

Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Prepare input branch
if: ${{ github.event.inputs.branch != "" }}
if: ${{ github.event.inputs.branch != '' }}
run: echo "branch=refs/heads/${{ github.event.inputs.branch }}" >> $GITHUB_ENV

- name: Checkout Repository
Expand Down Expand Up @@ -70,9 +70,9 @@ jobs:
TAGS+=",$name:latest"
echo "TAGS=$TAGS" >> $GITHUB_ENV

- name: Set the tag on wokflow dispatch
- name: Set the tag on workflow dispatch
if: ${{ github.ref_type != 'tag' }}
run: echo "TAGS=$(git rev-parse --short HEAD)" >> $GITHUB_ENV
run: echo "TAGS=voltaserve/api:$(git rev-parse --short HEAD)" >> $GITHUB_ENV

- name: Build and Push Docker Image
uses: docker/build-push-action@v5
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/build-conversion.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Prepare input branch
if: ${{ github.event.inputs.branch != "" }}
if: ${{ github.event.inputs.branch != '' }}
run: echo "branch=refs/heads/${{ github.event.inputs.branch }}" >> $GITHUB_ENV

- name: Checkout Repository
Expand Down Expand Up @@ -70,9 +70,9 @@ jobs:
TAGS+=",$name:latest"
echo "TAGS=$TAGS" >> $GITHUB_ENV

- name: Set the tag on wokflow dispatch
- name: Set the tag on workflow dispatch
if: ${{ github.ref_type != 'tag' }}
run: echo "TAGS=$(git rev-parse --short HEAD)" >> $GITHUB_ENV
run: echo "TAGS=voltaserve/conversion:$(git rev-parse --short HEAD)" >> $GITHUB_ENV

- name: Build and Push Docker Image
uses: docker/build-push-action@v5
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build-idp.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ jobs:
TAGS+=",$name:latest"
echo "TAGS=$TAGS" >> $GITHUB_ENV

- name: Set the tag on wokflow dispatch
- name: Set the tag on workflow dispatch
if: ${{ github.ref_type != 'tag' }}
run: echo "TAGS=$(git rev-parse --short HEAD)" >> $GITHUB_ENV

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build-language.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ jobs:
TAGS+=",$name:latest"
echo "TAGS=$TAGS" >> $GITHUB_ENV

- name: Set the tag on wokflow dispatch
- name: Set the tag on workflow dispatch
if: ${{ github.ref_type != 'tag' }}
run: echo "TAGS=$(git rev-parse --short HEAD)" >> $GITHUB_ENV

Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/build-mosaic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Prepare input branch
if: ${{ github.event.inputs.branch != "" }}
if: ${{ github.event.inputs.branch != '' }}
run: echo "branch=refs/heads/${{ github.event.inputs.branch }}" >> $GITHUB_ENV

- name: Checkout Repository
Expand Down Expand Up @@ -70,9 +70,9 @@ jobs:
TAGS+=",$name:latest"
echo "TAGS=$TAGS" >> $GITHUB_ENV

- name: Set the tag on wokflow dispatch
- name: Set the tag on workflow dispatch
if: ${{ github.ref_type != 'tag' }}
run: echo "TAGS=$(git rev-parse --short HEAD)" >> $GITHUB_ENV
run: echo "TAGS=voltaserve/mosaic:$(git rev-parse --short HEAD)" >> $GITHUB_ENV

- name: Build and Push Docker Image
uses: docker/build-push-action@v5
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/build-ui.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Prepare input branch
if: ${{ github.event.inputs.branch != "" }}
if: ${{ github.event.inputs.branch != '' }}
run: echo "branch=refs/heads/${{ github.event.inputs.branch }}" >> $GITHUB_ENV

- name: Checkout Repository
Expand Down Expand Up @@ -70,9 +70,9 @@ jobs:
TAGS+=",$name:latest"
echo "TAGS=$TAGS" >> $GITHUB_ENV

- name: Set the tag on wokflow dispatch
- name: Set the tag on workflow dispatch
if: ${{ github.ref_type != 'tag' }}
run: echo "TAGS=$(git rev-parse --short HEAD)" >> $GITHUB_ENV
run: echo "TAGS=voltaserve/ui:$(git rev-parse --short HEAD)" >> $GITHUB_ENV

- name: Build and Push Docker Image
uses: docker/build-push-action@v5
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/build-webdav.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Prepare input branch
if: ${{ github.event.inputs.branch != "" }}
if: ${{ github.event.inputs.branch != '' }}
run: echo "branch=refs/heads/${{ github.event.inputs.branch }}" >> $GITHUB_ENV

- name: Checkout Repository
Expand Down Expand Up @@ -70,9 +70,9 @@ jobs:
TAGS+=",$name:latest"
echo "TAGS=$TAGS" >> $GITHUB_ENV

- name: Set the tag on wokflow dispatch
- name: Set the tag on workflow dispatch
if: ${{ github.ref_type != 'tag' }}
run: echo "TAGS=$(git rev-parse --short HEAD)" >> $GITHUB_ENV
run: echo "TAGS=voltaserve/webdav:$(git rev-parse --short HEAD)" >> $GITHUB_ENV

- name: Build and Push Docker Image
uses: docker/build-push-action@v5
Expand Down
1 change: 0 additions & 1 deletion api/.golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ linters:
- prealloc
- nestif
- lll
- gosec
- gocritic
- gocognit
- funlen
Expand Down
64 changes: 64 additions & 0 deletions api/infra/fixtures/join-organization.eml
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
Mime-Version: 1.0
Date: Now
From: "Voltaserve" <[email protected]>
To: "Someone" <[email protected]>
Subject: Invitation to join a Voltaserve organization
Content-Type: multipart/alternative;
boundary=XXX

--XXX
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain ; charset=UTF-8

You have been invited by Someone to join the organization ACME.
Please follow this link to sign up: example.com/sign-up
--XXX
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html; charset=UTF-8

<html>
<head>
<link rel=3D"preconnect" href=3D"https://fonts.googleapis.com" />
<link rel=3D"preconnect" href=3D"https://fonts.gstatic.com" crossorigin=
/>
<link
href=3D"https://fonts.googleapis.com/css2?family=3DIBM+Plex+Sans:ital=
,wght@0,100;0,200;0,300;0,400;0,500;0,600;0,700;1,100;1,200;1,300;1,400;1,5=
00;1,600;1,700&display=3Dswap"
rel=3D"stylesheet"
/>
<style>
.container {
font-family: "IBM Plex Sans", sans-serif;
font-size: 14px;
color: black;
width: 580px;
margin-left: auto;
margin-right: auto;
}
.link {
color: #0c4cf3 !important;
}
.link:visited {
color: #0c4cf3 !important;
}
</style>
</head>
<body>
<div class=3D"container">
<p>Hello,</p>
<p>
You have been invited by <b>Someone</b> to join the
organization <b>ACME</b>. Please follow this link to
view your incoming invitations:
</p>
<p>
<a class=3D"link" href=3D"example.com/account/invitation">
View incoming invitations
</a>
</p>
</div>
</body>
</html>

--XXX
63 changes: 63 additions & 0 deletions api/infra/fixtures/sign-up-and-join-organization.eml
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
Mime-Version: 1.0
Date: Now
From: "Voltaserve" <[email protected]>
To: "Someone" <[email protected]>
Subject: Invitation to join a Voltaserve organization
Content-Type: multipart/alternative;
boundary=XXX

--XXX
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain ; charset=UTF-8

You have been invited by Someone to join the organization ACME.
Please follow this link to view your incoming invitations: example.com/acco=
unt/incoming-invitations
--XXX
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html; charset=UTF-8

<html>
<head>
<link rel=3D"preconnect" href=3D"https://fonts.googleapis.com" />
<link rel=3D"preconnect" href=3D"https://fonts.gstatic.com" crossorigin=
/>
<link
href=3D"https://fonts.googleapis.com/css2?family=3DIBM+Plex+Sans:ital=
,wght@0,100;0,200;0,300;0,400;0,500;0,600;0,700;1,100;1,200;1,300;1,400;1,5=
00;1,600;1,700&display=3Dswap"
rel=3D"stylesheet"
/>
<style>
.container {
font-family: "IBM Plex Sans", sans-serif;
font-size: 14px;
color: black;
width: 580px;
margin-left: auto;
margin-right: auto;
}
.link {
color: #0c4cf3 !important;
}
.link:visited {
color: #0c4cf3 !important;
}
</style>
</head>
<body>
<div class=3D"container">
<p>Hello,</p>
<p>
You have been invited by <b>Someone</b> to join the
organization <b>ACME</b> in Voltaserve. Please follow
this link to sign up:
</p>
<p>
<a class=3D"link" href=3D"example.com/sign-up">Sign up</a>
</p>
</div>
</body>
</html>

--XXX
37 changes: 23 additions & 14 deletions api/infra/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"bytes"
"fmt"
"io"
"os"
"io/fs"
"path/filepath"
"text/template"

Expand All @@ -23,30 +23,39 @@ import (

"github.com/kouprlabs/voltaserve/api/config"
"github.com/kouprlabs/voltaserve/api/log"
"github.com/kouprlabs/voltaserve/api/templates"
)

type dialer interface {
DialAndSend(m ...*gomail.Message) error
}

type MessageParams struct {
Subject string
}

type MailTemplate struct {
dialer *gomail.Dialer
dialer dialer
config config.SMTPConfig
}

func NewMailTemplate() *MailTemplate {
mt := new(MailTemplate)
mt.config = config.GetConfig().SMTP
mt.dialer = gomail.NewDialer(mt.config.Host, mt.config.Port, mt.config.Username, mt.config.Password)
return mt
func NewMailTemplate(config config.SMTPConfig) *MailTemplate {
return NewMailTemplateWithDialer(config, gomail.NewDialer(config.Host, config.Port, config.Username, config.Password))
}

func NewMailTemplateWithDialer(config config.SMTPConfig, dialer dialer) *MailTemplate {
return &MailTemplate{
config: config,
dialer: dialer,
}
}

func (mt *MailTemplate) Send(templateName string, address string, variables map[string]string) error {
html, err := mt.getText(filepath.FromSlash("templates/"+templateName+"/template.html"), variables)
html, err := mt.getText(filepath.FromSlash(templateName+"/template.html"), variables)
if err != nil {
return err
}
text, err := mt.getText(filepath.FromSlash("templates/"+templateName+"/template.txt"), variables)
text, err := mt.getText(filepath.FromSlash(templateName+"/template.txt"), variables)
if err != nil {
return err
}
Expand All @@ -61,17 +70,17 @@ func (mt *MailTemplate) Send(templateName string, address string, variables map[
m.SetBody("text/plain ", text)
m.AddAlternative("text/html", html)
if err := mt.dialer.DialAndSend(m); err != nil {
return err
return fmt.Errorf("dial and sending mail: %w", err)
}
return nil
}

func (mt *MailTemplate) getText(path string, variables map[string]string) (string, error) {
f, err := os.Open(path)
f, err := templates.FS.Open(path)
Copy link
Collaborator Author

@dsonck92 dsonck92 Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that this will embed templates. Not sure if it was planned to allow the user to override the templates at some point. But if we talk about cloud native, we might want to bring this into some kind of specialized bucket in the future anyways so it can be edited online. And embed the defaults so it can be populated and/or reset.

But this was the easiest way (other than ignoring) to keep gosec happy. Gosec triggers because you may give .. in this path string, breaking out of the expected "root". One way to fix this is to use path.Join(root, path.Clean(path)) such that it's anchored to root, or the other is to give a virtual FS like done now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree about having the templates editable only, what about like exposing them to some great headless CMS like "Sanity".

if err != nil {
return "", err
}
defer func(f *os.File) {
defer func(f fs.File) {
if err := f.Close(); err != nil {
log.GetLogger().Error(err)
}
Expand All @@ -91,11 +100,11 @@ func (mt *MailTemplate) getText(path string, variables map[string]string) (strin
}

func (mt *MailTemplate) getMessageParams(templateName string) (*MessageParams, error) {
f, err := os.Open(filepath.FromSlash("templates/" + templateName + "/params.yml"))
f, err := templates.FS.Open(filepath.FromSlash(templateName + "/params.yml"))
if err != nil {
return nil, err
}
defer func(f *os.File) {
defer func(f fs.File) {
if err := f.Close(); err != nil {
log.GetLogger().Error(err)
}
Expand Down
Loading