While preparing the command, attempt to verify that the template
expansion happens in a way that will result in a non-shell-injection
command. I don't presume to say that this is a 100% prevention, and the
script itself can always do dumb shit with the file path later.
Nonetheless, we should make a best-effort attempt.
Equally, this could generate false positives for commands that are
strangely written but in fact safe. I think this is acceptable; external
versioning is currently used by approximately 0.02% of users, and
presumably most of them have a setup that is sane.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
Resp. directories for the database, and both dirs and symlinks for
`FileInfo.FileSize`. Instead handle it in the UI progress percentage. We
even already have special cases there for deletions, might as well
handle directories just like any other zero-sized needed item there.
This went through the very thorough testing of running it on my laptop,
the migration was applied and it seemed to be working fine after.
---------
Signed-off-by: Simon Frei <freisim93@gmail.com>
During initial folder scan and the scan is interrupted with a
context.Canceled error, it was previously logged as a "Failed initial
scan" error, which can be misleading
The change adds a explicit check for context.Canceled and silently
ignores it (fixes#10363)
Signed-off-by: Henrik Bråthen <henrikbs@proton.me>
Verify that block requests have a hash and that it's correct. This helps
prevent certain races and ensure that only expected data is ever
returned in response to a request.
(In Syncthing prior to 1.28.1 the block hash was omitted for encrypted
requests from trusted devices. This breaks compatibility with that
specific config on those versions.)
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
Ensure file was a file before the shortcut as well as after... (This was
implied when talking to a correct implementation, but not enforced.)
Make our file opening operations safe by default by ensuring the last
path component is not a symlink.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
We had a few places where we had perhaps too much of an opinion on the
permissions on created files and directories, sometimes fuled by a
misconception about how permissions work in both Unix and Windows. Recap
on the ground rules:
- On all unixes, all file & directory creation (`Mkdir`, `MkdirAll`,
`Create`, `WriteFile`, `Open`) has the given permission bits filtered
via the user's umask. The proper permissions for us to use are in almost
all cases 0o666 for files and 0o777 for directories, strange as that may
look at the call site.
- On Windows, there is no umask but in turn all of the permission bits
except the user write bit are ignored. The absence of user write bit is
converted into the read only attribute. This means that what is proper
for Unix above is also proper for Windows.
- We make an exception when creating files for certificate keys and the
config / database directories, as those contain secrets we think should remain closed
even if the user generally collaborates with other users on the system.
(Also removal of a bugfixed copy of MkdirAll for Windows that hasn't
been necessary for a few years.)
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
This change allows Syncthing to be launched from Explorer without
showing a console window, while preserving the existing command-line
behavior.
Previously, launching syncthing.exe from Explorer would always allocate
a console window, which could only be hidden later by using
`--no-console`. It was not possible to avoid console allocation entirely
without introducing other issues.
On Windows 24H2 and later a new application manifest allows us to
achieve it. See [console allocation
policy](https://learn.microsoft.com/en-us/windows/console/console-allocation-policy)
This manifest is built into a syso-file by `goversioninfo`, which is
already used to generate Windows resource files consumed by the Go
compiler.
**Note1:** On Windows 24H2 and later, no console is allocated when
Syncthing is launched from Explorer, even if `--no-console` is set to
`False`. It can still be used as a CLI tool as usual if you call it from
console.
**Note2:** The content of the manifest file may not be formatted. Even a
`newline` would break it.
### Testing
Tested on Windows 11 25H2: No console visible from explorer. CLI works
as usual.
Ref #8046, ref #10633, ref #10481, ref #10600
Signed-off-by: Elias <1elias.bauer@gmail.com>
Co-authored-by: Elias <1elias.bauer@gmail.com>
### Purpose
On Windows replace `cmd.exe /C start` with direct `ShellExecute` API for
opening the webpage.
The previous implementation used `exec.Command("cmd.exe", "/C", "start
"+url)` which spawns two extra processes (cmd.exe → start). Launching
cmd.exe resulted in a shortly visible terminal.
Both
-`start`
-and another alternative `exec.Command("rundll32",
"url.dll,FileProtocolHandler", url).Start()`
are just wrappers for `ShellExecute`. So this implementation is even
more direct
### Testing
I executed the compiled syncthing.exe on Windows 11, both from explorer
and console. The webpage opened as expected.
### Screenshots
N/A.
### Documentation
N/A
## Authorship
Name: Elias @Shablone
Email: [1elias.bauer@gmail.com](mailto:1elias.bauer@gmail.com)
Signed-off-by: Elias <1elias.bauer@gmail.com>
Co-authored-by: Elias <1elias.bauer@gmail.com>
- Allow zero-sized requests since they are sent by all current versions
of Syncthing.
- Stop sending zero-sized requests since that's stupid.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
### Purpose
`path` is for slash-separated paths (URLs, BEP protocol); local file
system paths should use `path/filepath`. Fixed in
`cmd/stdiscosrv/database.go` (3 sites) and
`internal/db/sqlite/db_test.go` (1 site).
### Testing
`go build ./cmd/stdiscosrv/...` and `go vet` pass.
Signed-off-by: Yasuhiro Matsumoto <mattn.jp@gmail.com>
Fix a response body leak in `githubSourceCodeLoader.Load` where the body
was not closed when the HTTP status was non-200.
Signed-off-by: Yasuhiro Matsumoto <mattn.jp@gmail.com>
lib/ur brings in a lot of dependencies we don't need in e.g.
stcrashreceiver, who only needs the small failure reporting structs.
Make those part of the lean `contract` package instead.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
The runtime prints a lot of context for crashes due to bad pointers etc,
which is required to understand the crash, but this context comes before
the `fatal error: ...` line. Currently those lines get filtered out and
not included in the crash report. This change modifies the criteria so
that we start collecting crash data also at a line that begins with
`runtime:`, and tweaks the parsing later to look for the specific
`panic:` or `fatal error:` which may come later as the subject.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
This makes sure the user running Syncthing, and hence Synchting itself,
has read/write/execute on directories in .stversions. The other
permission bits remain copied from the source directory, ensuring
whatever group and other permissions were set remain in effect.
Closes#10695.
---------
Signed-off-by: Jakob Borg <jakob@kastelo.net>
The locking logic for upgrades got inverted in the lockfile changes. If
we got the lock it means Syncthing wasn't already running, so we can do
a direct upgrade. If we failed to get the lock it means Syncthing was
running and we should tell the REST interface to do the upgrade.
Signed-off-by: Jakob Borg <jakob@kastelo.net>
* infrastructure:
fix(stcrashreceiver): allow extra pre/post data in version line
chore(stcrashreceiver): improve logging
chore(stdiscosrv): prewarm counters at startup
